From 441789be5b83ce7d6c26151436673fd0b01ca6a9 Mon Sep 17 00:00:00 2001 From: Joshua Peraza Date: Tue, 30 Jan 2018 12:20:03 -0800 Subject: [PATCH] android: Implement Semaphore with a condition variable and mutex Bionic uses negative values of a semaphore to represent contention. `sem_timedwait` fails to restore the value to 0 on timeout resulting in an error (EBUSY) upon calling `sem_destroy`. Bug: crashpad:30 Change-Id: If1c73a54a879ebd003b0792ebb8f68ceb83ac8bb Reviewed-on: https://chromium-review.googlesource.com/894106 Reviewed-by: Mark Mentovai Commit-Queue: Joshua Peraza --- util/synchronization/semaphore.h | 7 +++++ util/synchronization/semaphore_posix.cc | 42 +++++++++++++++++++++++-- util/synchronization/semaphore_test.cc | 5 ++- 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/util/synchronization/semaphore.h b/util/synchronization/semaphore.h index 49479639..dc3000b0 100644 --- a/util/synchronization/semaphore.h +++ b/util/synchronization/semaphore.h @@ -23,6 +23,9 @@ #include #elif defined(OS_WIN) #include +#elif defined(OS_ANDROID) +#include +#include #else #include #endif @@ -77,6 +80,10 @@ class Semaphore { dispatch_semaphore_t semaphore_; #elif defined(OS_WIN) HANDLE semaphore_; +#elif defined(OS_ANDROID) + std::condition_variable cv_; + std::mutex mutex_; + int value_; #else sem_t semaphore_; #endif diff --git a/util/synchronization/semaphore_posix.cc b/util/synchronization/semaphore_posix.cc index 59ed71f2..3875f3f4 100644 --- a/util/synchronization/semaphore_posix.cc +++ b/util/synchronization/semaphore_posix.cc @@ -18,13 +18,51 @@ #include #include +#include + #include "base/logging.h" #include "base/posix/eintr_wrapper.h" #include "util/misc/time.h" namespace crashpad { -#if !defined(OS_MACOSX) +#if defined(OS_ANDROID) + +Semaphore::Semaphore(int value) : cv_(), mutex_(), value_(value) {} + +Semaphore::~Semaphore() = default; + +void Semaphore::Wait() { + std::unique_lock lock(mutex_); + cv_.wait(lock, [this] { return this->value_ > 0; }); + --value_; +} + +bool Semaphore::TimedWait(double seconds) { + DCHECK_GE(seconds, 0.0); + + if (isinf(seconds)) { + Wait(); + return true; + } + + std::unique_lock lock(mutex_); + if (!cv_.wait_for(lock, std::chrono::duration(seconds), [this] { + return this->value_ > 0; + })) { + return false; + } + --value_; + return true; +} + +void Semaphore::Signal() { + std::lock_guard lock(mutex_); + ++value_; + cv_.notify_one(); +} + +#elif !defined(OS_MACOSX) Semaphore::Semaphore(int value) { PCHECK(sem_init(&semaphore_, 0, value) == 0) << "sem_init"; @@ -65,6 +103,6 @@ void Semaphore::Signal() { PCHECK(sem_post(&semaphore_) == 0) << "sem_post"; } -#endif +#endif // OS_ANDROID } // namespace crashpad diff --git a/util/synchronization/semaphore_test.cc b/util/synchronization/semaphore_test.cc index e1f3648c..10f7546d 100644 --- a/util/synchronization/semaphore_test.cc +++ b/util/synchronization/semaphore_test.cc @@ -41,7 +41,10 @@ TEST(Semaphore, TimedWait) { TEST(Semaphore, TimedWaitTimeout) { Semaphore semaphore(0); - EXPECT_FALSE(semaphore.TimedWait(0.01)); // 10ms + semaphore.Signal(); + constexpr double kTenMs = 0.01; + EXPECT_TRUE(semaphore.TimedWait(kTenMs)); + EXPECT_FALSE(semaphore.TimedWait(kTenMs)); } TEST(Semaphore, TimedWaitInfinite_0) {