From 6c5a7cc5fa018ebb07bd5cd7c07d941607df6f8a Mon Sep 17 00:00:00 2001 From: Danny Robson Date: Wed, 15 Aug 2018 17:00:29 +1000 Subject: [PATCH] thread/semaphore: specalise the implementation for win32 --- test/thread/semaphore.cpp | 7 ++++ thread/semaphore.hpp | 4 +- thread/semaphore_win32.cpp | 75 +++++++++++++++++++++----------------- thread/semaphore_win32.hpp | 27 +++++++------- 4 files changed, 65 insertions(+), 48 deletions(-) diff --git a/test/thread/semaphore.cpp b/test/thread/semaphore.cpp index 8c9ac496..8d3e3238 100644 --- a/test/thread/semaphore.cpp +++ b/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 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"); diff --git a/thread/semaphore.hpp b/thread/semaphore.hpp index 9ee2afbe..a7e4acc8 100644 --- a/thread/semaphore.hpp +++ b/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 diff --git a/thread/semaphore_win32.cpp b/thread/semaphore_win32.cpp index 0d04fe02..9e829aae 100644 --- a/thread/semaphore_win32.cpp +++ b/thread/semaphore_win32.cpp @@ -8,8 +8,12 @@ #include "semaphore_win32.hpp" +#include "../win32/except.hpp" + #include "../debug.hpp" +#include + 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::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 (); } diff --git a/thread/semaphore_win32.hpp b/thread/semaphore_win32.hpp index 0205ef7e..0d0ab794 100644 --- a/thread/semaphore_win32.hpp +++ b/thread/semaphore_win32.hpp @@ -8,16 +8,18 @@ #pragma once +#include "../win32/handle.hpp" + #include -#include -#include 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 m_value; - std::mutex m_mutex; - std::condition_variable m_cv; + std::atomic m_value; + win32::handle m_handle; }; };