thread/semaphore: specalise the implementation for win32

test/thread/semaphore.cpp

@@ -28,9 +28,12 @@ main ()
         tap.expect_eq (sem.lock (), 0, "bare acquire decrements without blocking");
     }
 
+    // test that two threads can cooperate on a single semaphore.
     {
         cruft::thread::semaphore sem (1);
 
+        // the spawned thread attempts to double acquire a semaphore with
+        // only one available and so should get blocked immediately.
         std::atomic<int> test = 0;
         std::thread t ([&] () {
             sem.lock ();
@@ -38,11 +41,15 @@ main ()
             test = 1;
         });
 
+        // wait until we know the thread should have been blocked. it should
+        // not have touched the 'test' variable at this point.
         while (sem.value () > 0)
             std::this_thread::yield ();
 
         tap.expect_eq (test, 0, "locking blocks in foreign thread");
 
+        // unlock the semaphore, wait for the thread to finish, and check it
+        // touched the 'test' variable.
         sem.unlock ();
         t.join ();
         tap.expect_eq (test, 1, "unlocking resumes foreign thread");

thread/semaphore.hpp

@@ -2,6 +2,8 @@
 
 #if defined(PLATFORM_LINUX)
 #include "semaphore_linux.hpp"
-#else
+#elif defined(PLATFORM_WIN32)
 #include "semaphore_win32.hpp"
+#else
+#error "Unsupported threading platform"
 #endif

thread/semaphore_win32.cpp

@@ -8,8 +8,12 @@
 
 #include "semaphore_win32.hpp"
 
+#include "../win32/except.hpp"
+
 #include "../debug.hpp"
 
+#include <windows.h>
+
 using cruft::thread::semaphore;
 
 
@@ -20,38 +24,39 @@ semaphore::semaphore ():
 
 
 //-----------------------------------------------------------------------------
-semaphore::semaphore (int initial):
-    m_value (initial)
-{ ; }
+semaphore::semaphore (value_type initial):
+    m_handle (
+        win32::error::try_call (
+            CreateSemaphore,
+            nullptr,
+            initial,
+            std::numeric_limits<LONG>::max (),
+            nullptr
+        )
+    )
+{
+    m_value = initial;
+}
 
 
 ///////////////////////////////////////////////////////////////////////////////
-int
-semaphore::acquire (int count)
+semaphore::value_type
+semaphore::acquire (value_type const count)
 {
     CHECK_GE (count, 0);
 
-    do {
-        int now = m_value;
+    for (value_type i = 0; i < count; ++i) {
+        if (WAIT_OBJECT_0 != WaitForSingleObject(m_handle.native(), INFINITE))
+            win32::error::throw_code();
+        --m_value;
+    }
 
-        // if our value is positive then attempt to decrement it and return,
-        // else retry because someone interfered with us.
-        if (now - count >= 0) {
-            if (m_value.compare_exchange_weak (now, now - count))
-                return now - count;
-            continue;
-        }
-
-        // the count doesn't appear to allow us to acquire. sleep until
-        // there's been a modification and retry.
-        std::unique_lock lk (m_mutex);
-        m_cv.wait (lk, [&, this] () { return m_value - count >= 0; });
-    } while (1);
+    return m_value;
 }
 
 
 //-----------------------------------------------------------------------------
-int
+semaphore::value_type
 semaphore::acquire (void)
 {
     return acquire (1);
@@ -59,18 +64,22 @@ semaphore::acquire (void)
 
 
 //-----------------------------------------------------------------------------
-int
-semaphore::release (int count)
+semaphore::value_type
+semaphore::release (value_type count)
 {
-    auto res = m_value += count;
-    if (res > 0)
-        m_cv.notify_one();
-    return res;
+    LONG previous;
+    for (value_type i = 0; i < count; ++i) {
+        if (!ReleaseSemaphore (m_handle.native(), count, &previous))
+            win32::error::throw_code ();
+        ++m_value;
+    }
+
+    return previous + 1;
 }
 
 
 //-----------------------------------------------------------------------------
-int
+semaphore::value_type
 semaphore::release (void)
 {
     return release (1);
@@ -78,7 +87,7 @@ semaphore::release (void)
 
 
 ///////////////////////////////////////////////////////////////////////////////
-int
+semaphore::value_type
 semaphore::value (void) const
 {
     return m_value;
@@ -86,7 +95,7 @@ semaphore::value (void) const
 
 
 //-----------------------------------------------------------------------------
-int
+semaphore::value_type
 semaphore::operator++ (void)
 {
     return release ();
@@ -94,10 +103,8 @@ semaphore::operator++ (void)
 
 
 //-----------------------------------------------------------------------------
-int
+semaphore::value_type
 semaphore::operator-- (void)
 {
-    // we don't need to wake anyone because this will only serve to delay
-    // their wakeup.
-    return --m_value;
+    return acquire ();
 }

thread/semaphore_win32.hpp

@@ -8,16 +8,18 @@
 
 #pragma once
 
+#include "../win32/handle.hpp"
+
 #include <atomic>
-#include <condition_variable>
-#include <mutex>
 
 
 namespace cruft::thread {
     /// Explicitly does not conform to BasicLockable.
     class semaphore {
     public:
-        semaphore (int initial);
+        using value_type = LONG;
+
+        semaphore (value_type initial);
         semaphore ();
 
         semaphore (const semaphore&) = delete;
@@ -25,22 +27,21 @@ namespace cruft::thread {
         semaphore (semaphore&&) = delete;
         semaphore& operator= (semaphore&&) = delete;
 
-        int acquire (void);
-        int acquire (int count);
-        int release (void);
-        int release (int count);
+        value_type acquire (void);
+        value_type acquire (value_type count);
+        value_type release (void);
+        value_type release (value_type count);
 
         auto   lock (void) { return acquire (); }
         auto unlock (void) { return release (); }
 
-        int value (void) const;
+        value_type value (void) const;
 
-        int operator++ (void);
-        int operator-- (void);
+        value_type operator++ (void);
+        value_type operator-- (void);
 
     private:
-        std::atomic<int> m_value;
-        std::mutex m_mutex;
-        std::condition_variable m_cv;
+        std::atomic<LONG> m_value;
+        win32::handle m_handle;
     };
 };