summaryrefslogtreecommitdiff
path: root/osi
diff options
context:
space:
mode:
authorAndre Eisenbach <eisenbach@google.com>2015-12-22 17:50:24 -0800
committerAndre Eisenbach <eisenbach@google.com>2015-12-22 19:09:52 -0800
commit48df46b8398cf224a5209002cce3d534c4480d99 (patch)
tree9ce8e0e1035c385e117a9160849242e5c0c02d6a /osi
parentd0aa6cc53abaf122a2426c20691ccfa025ca7369 (diff)
downloadandroid-system-bt-48df46b8398cf224a5209002cce3d534c4480d99.tar.gz
android-system-bt-48df46b8398cf224a5209002cce3d534c4480d99.tar.xz
Properly reset NONBLOCK flag in semaphore_try_wait()
Without this fix, calling semaphore_try_wait() on a semaphore that wasn't currently set, would leave the NONBLOCK flag on the file descriptor as a side-effect. Also added a unit test for semaphores, including a test specifically for this condition. Change-Id: I0ea37bb68b14c76febaab25b3aee1bb4f5acee8c
Diffstat (limited to 'osi')
-rw-r--r--osi/Android.mk1
-rw-r--r--osi/src/semaphore.c7
-rw-r--r--osi/test/semaphore_test.cpp87
-rw-r--r--osi/test/thread_test.cpp1
4 files changed, 92 insertions, 4 deletions
diff --git a/osi/Android.mk b/osi/Android.mk
index c417505..40cedc2 100644
--- a/osi/Android.mk
+++ b/osi/Android.mk
@@ -67,6 +67,7 @@ btosiCommonTestSrc := \
./test/list_test.cpp \
./test/reactor_test.cpp \
./test/ringbuffer_test.cpp \
+ ./test/semaphore_test.cpp \
./test/thread_test.cpp \
./test/time_test.cpp
diff --git a/osi/src/semaphore.c b/osi/src/semaphore.c
index 8693c9c..59b250a 100644
--- a/osi/src/semaphore.c
+++ b/osi/src/semaphore.c
@@ -85,13 +85,14 @@ bool semaphore_try_wait(semaphore_t *semaphore) {
return false;
}
+ bool rc = true;
eventfd_t value;
if (eventfd_read(semaphore->fd, &value) == -1)
- return false;
+ rc = false;
if (fcntl(semaphore->fd, F_SETFL, flags) == -1)
- LOG_ERROR(LOG_TAG, "%s unable to resetore flags for semaphore fd: %s", __func__, strerror(errno));
- return true;
+ LOG_ERROR(LOG_TAG, "%s unable to restore flags for semaphore fd: %s", __func__, strerror(errno));
+ return rc;
}
void semaphore_post(semaphore_t *semaphore) {
diff --git a/osi/test/semaphore_test.cpp b/osi/test/semaphore_test.cpp
new file mode 100644
index 0000000..c30dd0b
--- /dev/null
+++ b/osi/test/semaphore_test.cpp
@@ -0,0 +1,87 @@
+#include <gtest/gtest.h>
+
+#include "AllocationTestHarness.h"
+
+extern "C" {
+#include <unistd.h>
+#include <sys/select.h>
+
+#include "osi/include/osi.h"
+#include "osi/include/reactor.h"
+#include "osi/include/semaphore.h"
+#include "osi/include/thread.h"
+}
+
+struct SemaphoreTestSequenceHelper {
+ semaphore_t *semaphore;
+ int counter;
+};
+
+namespace {
+ void sleep_then_increment_counter(void *context) {
+ SemaphoreTestSequenceHelper *helper =
+ reinterpret_cast<SemaphoreTestSequenceHelper*>(context);
+ assert(helper);
+ assert(helper->semaphore);
+ sleep(1);
+ ++helper->counter;
+ semaphore_post(helper->semaphore);
+ }
+} // namespace
+
+class SemaphoreTest : public AllocationTestHarness {};
+
+TEST_F(SemaphoreTest, test_new_simple) {
+ semaphore_t *semaphore = semaphore_new(0);
+ ASSERT_TRUE(semaphore != NULL);
+ semaphore_free(semaphore);
+}
+
+TEST_F(SemaphoreTest, test_new_with_value) {
+ semaphore_t *semaphore = semaphore_new(3);
+ ASSERT_TRUE(semaphore != NULL);
+
+ EXPECT_TRUE(semaphore_try_wait(semaphore));
+ EXPECT_TRUE(semaphore_try_wait(semaphore));
+ EXPECT_TRUE(semaphore_try_wait(semaphore));
+ EXPECT_FALSE(semaphore_try_wait(semaphore));
+
+ semaphore_free(semaphore);
+}
+
+TEST_F(SemaphoreTest, test_try_wait) {
+ semaphore_t *semaphore = semaphore_new(0);
+ ASSERT_TRUE(semaphore != NULL);
+
+ EXPECT_FALSE(semaphore_try_wait(semaphore));
+ semaphore_post(semaphore);
+ EXPECT_TRUE(semaphore_try_wait(semaphore));
+ EXPECT_FALSE(semaphore_try_wait(semaphore));
+
+ semaphore_free(semaphore);
+}
+
+TEST_F(SemaphoreTest, test_wait_after_post) {
+ semaphore_t *semaphore = semaphore_new(0);
+ ASSERT_TRUE(semaphore != NULL);
+ semaphore_post(semaphore);
+ semaphore_wait(semaphore);
+ semaphore_free(semaphore);
+}
+
+TEST_F(SemaphoreTest, test_ensure_wait) {
+ semaphore_t *semaphore = semaphore_new(0);
+ ASSERT_TRUE(semaphore != NULL);
+ thread_t *thread = thread_new("semaphore_test_thread");
+ ASSERT_TRUE(thread != NULL);
+
+ EXPECT_FALSE(semaphore_try_wait(semaphore));
+ SemaphoreTestSequenceHelper sequence_helper = {semaphore, 0};
+ thread_post(thread, sleep_then_increment_counter, &sequence_helper);
+ semaphore_wait(semaphore);
+ EXPECT_EQ(sequence_helper.counter, 1) <<
+ "semaphore_wait() did not wait for counter to increment";
+
+ semaphore_free(semaphore);
+ thread_free(thread);
+}
diff --git a/osi/test/thread_test.cpp b/osi/test/thread_test.cpp
index d624d7d..b45365d 100644
--- a/osi/test/thread_test.cpp
+++ b/osi/test/thread_test.cpp
@@ -6,7 +6,6 @@ extern "C" {
#include <sys/select.h>
#include "osi/include/reactor.h"
-#include "osi/include/semaphore.h"
#include "osi/include/thread.h"
#include "osi/include/osi.h"
}