From 46adb81764b20f434eb124cfe49002be040b2227 Mon Sep 17 00:00:00 2001 From: Christian Kern Date: Mon, 9 Nov 2015 14:54:31 +0100 Subject: [PATCH] Worked in review comments for ticket EMBB453 --- base_c/include/embb/base/c/mutex.h | 2 +- base_c/src/mutex.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ base_c/test/shared_mutex_test.cc | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- base_c/test/shared_mutex_test.h | 8 ++++++++ base_cpp/src/mutex.cc | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------- base_cpp/test/shared_mutex_test.cc | 16 +++------------- 6 files changed, 372 insertions(+), 33 deletions(-) diff --git a/base_c/include/embb/base/c/mutex.h b/base_c/include/embb/base/c/mutex.h index 863a76f..fc31293 100644 --- a/base_c/include/embb/base/c/mutex.h +++ b/base_c/include/embb/base/c/mutex.h @@ -314,4 +314,4 @@ void embb_shared_mutex_destroy( * \} */ -#endif /* EMBB_BASE_C_MUTEX_H_ */ +#endif // EMBB_BASE_C_MUTEX_H_ diff --git a/base_c/src/mutex.c b/base_c/src/mutex.c index cd32696..8848138 100644 --- a/base_c/src/mutex.c +++ b/base_c/src/mutex.c @@ -115,3 +115,121 @@ void embb_mutex_destroy(embb_mutex_t* mutex) { } #endif /* EMBB_PLATFORM_THREADING_POSIXTHREADS */ + +#ifdef EMBB_PLATFORM_THREADING_WINTHREADS + +int embb_shared_mutex_init(embb_shared_mutex_t* shared_mutex) { + InitializeSRWLock(shared_mutex); + return EMBB_SUCCESS; +} + +int embb_shared_mutex_lock(embb_shared_mutex_t* shared_mutex) { + AcquireSRWLockExclusive(shared_mutex); + return EMBB_SUCCESS; +} + +int embb_shared_mutex_try_lock(embb_shared_mutex_t* shared_mutex) { + BOOLEAN success; + success = TryAcquireSRWLockExclusive(shared_mutex); + if (success == 0) return EMBB_BUSY; + return EMBB_SUCCESS; +} + +int embb_shared_mutex_unlock(embb_shared_mutex_t* shared_mutex) { + ReleaseSRWLockExclusive(shared_mutex); + return EMBB_SUCCESS; +} + +int embb_shared_mutex_lock_shared(embb_shared_mutex_t* shared_mutex) { + AcquireSRWLockShared(shared_mutex); + return EMBB_SUCCESS; +} + +int embb_shared_mutex_try_lock_shared(embb_shared_mutex_t* shared_mutex) { + BOOLEAN success; + success = TryAcquireSRWLockShared(shared_mutex); + if (success == 0) return EMBB_BUSY; + return EMBB_SUCCESS; +} + +int embb_shared_mutex_unlock_shared(embb_shared_mutex_t* shared_mutex) { + ReleaseSRWLockShared(shared_mutex); + return EMBB_SUCCESS; +} + +void embb_shared_mutex_destroy(embb_shared_mutex_t* shared_mutex) { + // Quoting MSDN: "SRW locks do not need to be explicitly destroyed". + EMBB_UNUSED(shared_mutex); +} + +#endif /* EMBB_PLATFORM_THREADING_WINTHREADS */ + +#ifdef EMBB_PLATFORM_THREADING_POSIXTHREADS + +int embb_shared_mutex_init(embb_shared_mutex_t* shared_mutex) { + int result = pthread_rwlock_init(shared_mutex, NULL); + if (result != 0) { + return EMBB_ERROR; + } + return EMBB_SUCCESS; +} + +int embb_shared_mutex_lock(embb_shared_mutex_t* shared_mutex) { + int result = pthread_rwlock_wrlock(shared_mutex); + if (result != 0) { + return EMBB_ERROR; + } + return EMBB_SUCCESS; +} + +int embb_shared_mutex_try_lock(embb_shared_mutex_t* shared_mutex) { + int result = pthread_rwlock_trywrlock(shared_mutex); + if (result == 0) { + return EMBB_SUCCESS; + } + if (result == EBUSY) { + return EMBB_BUSY; + } + return EMBB_ERROR; +} + +int embb_shared_mutex_unlock(embb_shared_mutex_t* shared_mutex) { + int result = pthread_rwlock_unlock(shared_mutex); + if (result != 0) { + return EMBB_ERROR; + } + return EMBB_SUCCESS; +} + +int embb_shared_mutex_lock_shared(embb_shared_mutex_t* shared_mutex) { + int result = pthread_rwlock_rdlock(shared_mutex); + if (result != 0) { + return EMBB_ERROR; + } + return EMBB_SUCCESS; +} + +int embb_shared_mutex_try_lock_shared(embb_shared_mutex_t* shared_mutex) { + int result = pthread_rwlock_tryrdlock(shared_mutex); + if (result == 0) { + return EMBB_SUCCESS; + } + if (result == EBUSY) { + return EMBB_BUSY; + } + return EMBB_ERROR; +} + +int embb_shared_mutex_unlock_shared(embb_shared_mutex_t* shared_mutex) { + int result = pthread_rwlock_unlock(shared_mutex); + if (result != 0) { + return EMBB_ERROR; + } + return EMBB_SUCCESS; +} + +void embb_shared_mutex_destroy(embb_shared_mutex_t* shared_mutex) { + pthread_rwlock_destroy(shared_mutex); +} + +#endif /* EMBB_PLATFORM_THREADING_POSIXTHREADS */ diff --git a/base_c/test/shared_mutex_test.cc b/base_c/test/shared_mutex_test.cc index 63599e3..e9e1fc1 100644 --- a/base_c/test/shared_mutex_test.cc +++ b/base_c/test/shared_mutex_test.cc @@ -30,9 +30,8 @@ namespace embb { namespace base { namespace test { - SharedMutexTest::SharedMutexTest() - : shared_mutex_(), + : shared_mutex_(), counter_(0), num_threads_(partest::TestSuite::GetDefaultNumThreads()), num_iterations_(partest::TestSuite::GetDefaultNumIterations()) { @@ -48,6 +47,123 @@ SharedMutexTest::SharedMutexTest() .Add(&SharedMutexTest::TestExclusiveWriterWriterMethod, this, num_threads_ / 2, num_iterations_) .Post(&SharedMutexTest::TestExclusiveWriterPost, this); + + CreateUnit("Basic test: read lock after write lock fails") + .Pre(&SharedMutexTest::TestLockedPre, this) + .Add(&SharedMutexTest::TestLockedForWritingPreventsLockForReading + , this, 2, num_iterations_) + .Add(&SharedMutexTest::TestLockedPost, this); + + CreateUnit("Basic test: write lock after read lock fails") + .Pre(&SharedMutexTest::TestLockedPre, this) + .Add(&SharedMutexTest::TestLockedForReadingPreventsLockForWriting, this, 2, + num_iterations_) + .Add(&SharedMutexTest::TestLockedPost, this); +} + +void SharedMutexTest::TestLockedPre() { + embb_atomic_store_int(&synchronize_, 0); + int success = embb_shared_mutex_init(&shared_mutex_); + + PT_ASSERT_EQ_MSG(success, EMBB_SUCCESS, "Failed to initialize shared mutex."); +} + +void SharedMutexTest::TestLockedForWritingPreventsLockForReading() { + int expected = 0; + int success = 0; + int which_thread = 0; + + if (embb_atomic_compare_and_swap_int(&synchronize_, &expected, 1)) { + // we are the write locking thread (will happen first)! + success = embb_shared_mutex_lock(&shared_mutex_); + + PT_ASSERT_EQ_MSG(success, EMBB_SUCCESS, + "Failed to lock shared mutex for writing"); + + // signal the second thread to continue + embb_atomic_store_int(&synchronize_, 2); + } else { + while (embb_atomic_load_int(&synchronize_) != 2) {} + // we are the read lock thread! (second thread) + which_thread = 1; + + // the mutex is locked for writing... try lock for reading must fail now! + success = embb_shared_mutex_try_lock_shared(&shared_mutex_); + + PT_ASSERT_EQ_MSG(success, EMBB_BUSY, + "Not failed to lock shared mutex for reading"); + + // synchronize, that first thread can unlock + embb_atomic_store_int(&synchronize_, 3); + } + + if (which_thread == 0) { + // wait for second thread to finish! + while (embb_atomic_load_int(&synchronize_) != 3) {} + + // the first thread unlocks the mutex... + success = embb_shared_mutex_unlock(&shared_mutex_); + + PT_ASSERT_EQ_MSG(success, EMBB_SUCCESS, + "Failed to unlock mutex"); + + // reset synchronize flag for next round... + embb_atomic_store_int(&synchronize_, 0); + } else { + //wait for next round + while (embb_atomic_load_int(&synchronize_) == 3) {} + } +} + +void SharedMutexTest::TestLockedForReadingPreventsLockForWriting() { + int expected = 0; + int success = 0; + int which_thread = 0; + + if (embb_atomic_compare_and_swap_int(&synchronize_, &expected, 1)) { + // we are the write locking thread (will happen first)! + success = embb_shared_mutex_lock_shared(&shared_mutex_); + + PT_ASSERT_EQ_MSG(success, EMBB_SUCCESS, + "Failed to lock shared mutex for writing"); + + // signal the second thread to continue + embb_atomic_store_int(&synchronize_, 2); + } else { + while (embb_atomic_load_int(&synchronize_) != 2) {} + // we are the read lock thread! (second thread) + which_thread = 1; + + // the mutex is locked for writing... try lock for reading must fail now! + success = embb_shared_mutex_try_lock(&shared_mutex_); + + PT_ASSERT_EQ_MSG(success, EMBB_BUSY, + "Not failed to lock shared mutex for reading"); + + // synchronize, that first thread can unlock + embb_atomic_store_int(&synchronize_, 3); + } + + if (which_thread == 0) { + // wait for second thread to finish! + while (embb_atomic_load_int(&synchronize_) != 3) {} + + // the first thread unlocks the mutex... + success = embb_shared_mutex_unlock(&shared_mutex_); + + PT_ASSERT_EQ_MSG(success, EMBB_SUCCESS, + "Failed to unlock mutex"); + + // reset synchronize flag for next round... + embb_atomic_store_int(&synchronize_, 0); + } else { + //wait for next round + while (embb_atomic_load_int(&synchronize_) == 3) {} + } +} + +void SharedMutexTest::TestLockedPost() { + embb_shared_mutex_destroy(&shared_mutex_); } void SharedMutexTest::TestSharedReadPre() { @@ -105,7 +221,6 @@ void SharedMutexTest::TestExclusiveWriterPost() { "Counter value is inconsistent."); embb_shared_mutex_destroy(&shared_mutex_); } - } // namespace test } // namespace base } // namespace embb diff --git a/base_c/test/shared_mutex_test.h b/base_c/test/shared_mutex_test.h index def0098..1196fa8 100644 --- a/base_c/test/shared_mutex_test.h +++ b/base_c/test/shared_mutex_test.h @@ -30,6 +30,7 @@ #include #include #include +#include namespace embb { @@ -63,6 +64,13 @@ class SharedMutexTest : public partest::TestCase { void TestExclusiveWriterWriterMethod(); void TestExclusiveWriterPost(); + + void TestLockedForWritingPreventsLockForReading(); + void TestLockedForReadingPreventsLockForWriting(); + void TestLockedPost(); + void TestLockedPre(); + + embb_atomic_int synchronize_; embb_shared_mutex_t shared_mutex_; size_t counter_; size_t num_threads_; diff --git a/base_cpp/src/mutex.cc b/base_cpp/src/mutex.cc index 61693ba..79538a2 100644 --- a/base_cpp/src/mutex.cc +++ b/base_cpp/src/mutex.cc @@ -30,38 +30,146 @@ namespace embb { namespace base { namespace internal { + MutexBase::MutexBase(int mutex_type) : mutex_() { + embb_mutex_init(&mutex_, mutex_type); + } -MutexBase::MutexBase(int mutex_type) : mutex_() { - embb_mutex_init(&mutex_, mutex_type); + MutexBase::~MutexBase() { + embb_mutex_destroy(&mutex_); + } + + void MutexBase::Lock() { + embb_mutex_lock(&mutex_); + } + + bool MutexBase::TryLock() { + int result = embb_mutex_try_lock(&mutex_); + return result == EMBB_SUCCESS; + } + + void MutexBase::Unlock() { + embb_mutex_unlock(&mutex_); + } +} // namespace internal + +Mutex::Mutex() : MutexBase(EMBB_MUTEX_PLAIN) { } -MutexBase::~MutexBase() { - embb_mutex_destroy(&mutex_); +RecursiveMutex::RecursiveMutex() : MutexBase(EMBB_MUTEX_RECURSIVE) { } -void MutexBase::Lock() { - embb_mutex_lock(&mutex_); +SharedMutex::SharedMutex() + : shared_mutex_() { + int result = embb_shared_mutex_init(&shared_mutex_); + + switch (result) { + case EMBB_SUCCESS: + return; + case EMBB_ERROR: + EMBB_THROW(embb::base::ErrorException, "Error while initializing mutex."); + break; + default: + EMBB_THROW(embb::base::ErrorException, "Unknown error."); + break; + } } -bool MutexBase::TryLock() { - int result = embb_mutex_try_lock(&mutex_); - return result == EMBB_SUCCESS; +SharedMutex::~SharedMutex() { + embb_shared_mutex_destroy(&shared_mutex_); } -void MutexBase::Unlock() { - embb_mutex_unlock(&mutex_); +void SharedMutex::Lock() { + int result = embb_shared_mutex_lock(&shared_mutex_); + + switch (result) { + case EMBB_SUCCESS: + return; + case EMBB_ERROR: + EMBB_THROW(embb::base::ErrorException, "Error while acquiring mutex."); + break; + default: + EMBB_THROW(embb::base::ErrorException, "Unknown error."); + break; + } } -} // namespace internal +bool SharedMutex::TryLock() { + int result = embb_shared_mutex_try_lock(&shared_mutex_); -Mutex::Mutex() : MutexBase(EMBB_MUTEX_PLAIN) { + switch (result) { + case EMBB_SUCCESS: + return true; + case EMBB_BUSY: + return false; + case EMBB_ERROR: + EMBB_THROW(embb::base::ErrorException, "Error while acquiring mutex."); + break; + default: + EMBB_THROW(embb::base::ErrorException, "Unknown error."); + break; + } } -RecursiveMutex::RecursiveMutex() : MutexBase(EMBB_MUTEX_RECURSIVE) { +void SharedMutex::Unlock() { + int result = embb_shared_mutex_unlock(&shared_mutex_); + + switch (result) { + case EMBB_SUCCESS: + return; + case EMBB_ERROR: + EMBB_THROW(embb::base::ErrorException, "Error while releasing mutex."); + break; + default: + EMBB_THROW(embb::base::ErrorException, "Unknown error."); + break; + } } -} // namespace base -} // namespace embb +void SharedMutex::LockShared() { + int result = embb_shared_mutex_lock_shared(&shared_mutex_); + switch (result) { + case EMBB_SUCCESS: + return; + case EMBB_ERROR: + EMBB_THROW(embb::base::ErrorException, "Error while acquiring mutex."); + break; + default: + EMBB_THROW(embb::base::ErrorException, "Unknown error."); + break; + } +} +bool SharedMutex::TryLockShared() { + int result = embb_shared_mutex_try_lock_shared(&shared_mutex_); + switch (result) { + case EMBB_SUCCESS: + return true; + case EMBB_BUSY: + return false; + case EMBB_ERROR: + EMBB_THROW(embb::base::ErrorException, "Error while acquiring mutex."); + break; + default: + EMBB_THROW(embb::base::ErrorException, "Unknown error."); + break; + } +} + +void SharedMutex::UnlockShared() { + int result = embb_shared_mutex_unlock_shared(&shared_mutex_); + + switch (result) { + case EMBB_SUCCESS: + return; + case EMBB_ERROR: + EMBB_THROW(embb::base::ErrorException, "Error while releasing mutex."); + break; + default: + EMBB_THROW(embb::base::ErrorException, "Unknown error."); + break; + } +} +} // namespace base +} // namespace embb diff --git a/base_cpp/test/shared_mutex_test.cc b/base_cpp/test/shared_mutex_test.cc index 489d676..cbe5f58 100644 --- a/base_cpp/test/shared_mutex_test.cc +++ b/base_cpp/test/shared_mutex_test.cc @@ -31,7 +31,6 @@ namespace embb { namespace base { namespace test { - SharedMutexTest::SharedMutexTest() : shared_mutex_(), counter_(0), @@ -55,9 +54,6 @@ SharedMutexTest::SharedMutexTest() void SharedMutexTest::TestSharedReadThreadMethod() { SharedLock lock(shared_mutex_, embb::base::try_lock); PT_ASSERT_EQ_MSG(lock.OwnsLock(), true, "Failed to lock for reading."); - - int spin = 10000; - while (--spin != 0); } void SharedMutexTest::TestExclusiveWritePre() { @@ -67,11 +63,6 @@ void SharedMutexTest::TestExclusiveWritePre() { void SharedMutexTest::TestExclusiveWriteReaderMethod() { // Just add some contention SharedLock lock(shared_mutex_, embb::base::try_lock); - - if (lock.OwnsLock()) { - int spin = 10000; - while (--spin != 0); - } } void SharedMutexTest::TestExclusiveWriteWriterMethod() { @@ -113,20 +104,20 @@ void SharedMutexTest::TestSharedLockThreadMethod() { SharedLock<> lock(shared_mutex_, embb::base::defer_lock); PT_EXPECT_EQ(lock.OwnsLock(), false); } - + // Test try-lock construction { SharedLock<> lock(shared_mutex_, embb::base::try_lock); PT_EXPECT_EQ(lock.OwnsLock(), true); } - + // Test adopt lock construction { shared_mutex_.LockShared(); SharedLock<> lock(shared_mutex_, embb::base::adopt_lock); PT_EXPECT_EQ(lock.OwnsLock(), true); } - + // Test lock swapping { SharedMutex another_mutex; @@ -152,7 +143,6 @@ void SharedMutexTest::TestSharedLockThreadMethod() { PT_EXPECT_EQ(lock1.OwnsLock(), false); } } - } // namespace test } // namespace base } // namespace embb -- libgit2 0.26.0