Worked in review comments for ticket EMBB453

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_

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 */

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

base_c/test/shared_mutex_test.h

@@ -30,6 +30,7 @@
 #include <partest/partest.h>
 #include <embb/base/c/internal/platform.h>
 #include <embb/base/c/mutex.h>
+#include <embb/base/c/atomic.h>
 
 
 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_;

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

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<embb::base::SharedMutex> 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<embb::base::SharedMutex> 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