[runtime] Addressed PR1731 comments and...

- cleaned up private scoping
- removed generally unneeded API
- fix defect in unit tests causing stuck threads

Author: shawnce

include/swift/Runtime/Mutex.h

@@ -19,6 +19,8 @@
 
 #if (defined(__APPLE__) || defined(__linux__) || defined(__CYGWIN__))
 #define SWIFT_RUNTIME_MUTEX_HAVE_PHTREADS
+#else
+#error "Must implement the following if your platform doesn't support phtreads."
 #endif
 
 #ifdef SWIFT_RUNTIME_MUTEX_HAVE_PHTREADS
@@ -74,18 +76,15 @@ class MutexImpl {
   MutexImpl(MutexImpl &&) = delete;
   MutexImpl &operator=(MutexImpl &&) = delete;
 
-protected:
-  explicit MutexImpl(bool checked);
-
 public:
   void lock();
   void unlock();
   bool try_lock();
-
-public:
   void wait(Condition &condition);
 
 private:
+  explicit MutexImpl(bool checked);
+
 #ifdef SWIFT_RUNTIME_MUTEX_HAVE_PHTREADS
   pthread_mutex_t PThreadMutex;
 #endif
@@ -95,6 +94,7 @@ class MutexImpl {
 /// Use ScopedLock instead (see below).
 class ScopedLockImpl {
   friend class Mutex;
+  friend class ScopedLock;
 
 public:
   ScopedLockImpl() = delete;
@@ -105,20 +105,16 @@ class ScopedLockImpl {
   ScopedLockImpl(ScopedLockImpl &&) = delete;
   ScopedLockImpl &operator=(ScopedLockImpl &&) = delete;
 
-protected:
-  ScopedLockImpl(MutexImpl &impl) : Impl(impl) { Impl.lock(); }
-
-public:
-  void wait(Condition &condition) { Impl.wait(condition); }
-
 private:
+  ScopedLockImpl(MutexImpl &impl) : Impl(impl) { Impl.lock(); }
   MutexImpl &Impl;
 };
 
 /// Internal (private) implementation of scoped unlocking functionality.
 /// Use ScopedUnlock instead (see below).
 class ScopedUnlockImpl {
   friend class Mutex;
+  friend class ScopedUnlock;
 
 public:
   ScopedUnlockImpl() = delete;
@@ -129,10 +125,8 @@ class ScopedUnlockImpl {
   ScopedUnlockImpl(ScopedUnlockImpl &&) = delete;
   ScopedUnlockImpl &operator=(ScopedUnlockImpl &&) = delete;
 
-protected:
-  ScopedUnlockImpl(MutexImpl &impl) : Impl(impl) { Impl.unlock(); }
-
 private:
+  ScopedUnlockImpl(MutexImpl &impl) : Impl(impl) { Impl.unlock(); }
   MutexImpl &Impl;
 };
 
@@ -292,12 +286,6 @@ class ScopedLock : ScopedLockImpl {
   ScopedLock &operator=(const ScopedLock &) = delete;
   ScopedLock(ScopedLock &&) = delete;
   ScopedLock &operator=(ScopedLock &&) = delete;
-
-public:
-  /// Releases lock, waits on supplied condition, and relocks before returning.
-  ///
-  /// Precondition: Mutex locked by this thread, undefined otherwise.
-  void wait(Condition &condition) { ScopedLockImpl::wait(condition); }
 };
 
 /// A stack based object that unlocks the supplied mutex on construction

unittests/runtime/Mutex.cpp

@@ -285,10 +285,12 @@ TEST(MutexTest, ConditionThreaded) {
       [&](int index) {
         ScopedLock guard(mutex);
         while (true) {
-          if (count - index >= 50) {
-            count -= index;
-            mutex.unlock(); // Only to give other consumers a chance.
-            mutex.lock();
+          if (count > 50) {
+            count -= 1;
+            {
+              // To give other consumers a chance.
+              ScopedUnlock unguard(mutex);
+            }
             if (trace)
               printf("Consumer[%d] count-%d = %d\n", index, index, count);
             continue; // keep trying to consume before waiting again.
@@ -297,7 +299,7 @@ TEST(MutexTest, ConditionThreaded) {
               printf("Consumer[%d] count == %d and done!\n", index, count);
             break;
           }
-          guard.wait(condition);
+          mutex.wait(condition);
         }
       },
       [&](int index) {
@@ -328,10 +330,12 @@ TEST(MutexTest, ConditionLockOrWaitLockAndNotifyThreaded) {
       [&](int index) {
         mutex.lockOrWait(condition, [&, index] {
           while (true) {
-            if (count - index >= 50) {
-              count -= index;
-              mutex.unlock(); // Only to give other consumers a chance.
-              mutex.lock();
+            if (count > 50) {
+              count -= 1;
+              {
+                // To give other consumers a chance.
+                ScopedUnlock unguard(mutex);
+              }
               if (trace)
                 printf("Consumer[%d] count-%d = %d\n", index, index, count);
               continue; // keep trying to consume before waiting again.