arch: Use standard mutex utilities as underlying implementation

src/lib/arch/IArchMultithread.cpp

@@ -0,0 +1,51 @@
+/*
+ * barrier -- mouse and keyboard sharing utility
+ * Copyright (C) 2012-2016 Symless Ltd.
+ * Copyright (C) 2002 Chris Schoeneman
+ *
+ * This package is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * found in the file LICENSE that should have accompanied this file.
+ *
+ * This package is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "IArchMultithread.h"
+
+bool IArchMultithread::waitCondVar(ArchCond cond, ArchMutex mutex,
+                                   double timeout)
+{
+    std::unique_lock<std::mutex> lock{*mutex, std::adopt_lock};
+
+    // we can't wait on a condition variable and also wake it up for
+    // cancellation since we don't use posix cancellation.  so we
+    // must wake up periodically to check for cancellation.  we
+    // can't simply go back to waiting after the check since the
+    // condition may have changed and we'll have lost the signal.
+    // so we have to return to the caller.  since the caller will
+    // always check for spurious wakeups the only drawback here is
+    // performance:  we're waking up a lot more than desired.
+    static const double maxCancellationLatency = 0.1;
+    if (timeout < 0.0 || timeout > maxCancellationLatency) {
+        timeout = maxCancellationLatency;
+    }
+
+    // see if we should cancel this thread
+    testCancelThread();
+
+    auto ret = cond->wait_for(lock, seconds_to_chrono(timeout));
+    lock.release();
+
+    // check for cancel again
+    testCancelThread();
+
+    if (ret == std::cv_status::no_timeout)
+        return true;
+    return false;
+}

src/lib/arch/IArchMultithread.h

@@ -20,35 +20,11 @@
 
 #include "common/IInterface.h"
 #include <functional>
+#include <mutex>
+#include <condition_variable>
 
-/*!
+using ArchCond = std::condition_variable*;
-\class ArchCondImpl
+using ArchMutex = std::mutex*;
-\brief Internal condition variable data.
-An architecture dependent type holding the necessary data for a
-condition variable.
-*/
-class ArchCondImpl;
-
-/*!
-\var ArchCond
-\brief Opaque condition variable type.
-An opaque type representing a condition variable.
-*/
-typedef ArchCondImpl* ArchCond;
-
-/*!
-\class ArchMutexImpl
-\brief Internal mutex data.
-An architecture dependent type holding the necessary data for a mutex.
-*/
-class ArchMutexImpl;
-
-/*!
-\var ArchMutex
-\brief Opaque mutex type.
-An opaque type representing a mutex.
-*/
-typedef ArchMutexImpl* ArchMutex;
 
 /*!
 \class ArchThreadImpl
@@ -64,6 +40,11 @@ An opaque type representing a thread.
 */
 typedef ArchThreadImpl* ArchThread;
 
+inline std::chrono::nanoseconds seconds_to_chrono(double seconds)
+{
+    return std::chrono::nanoseconds(std::uint64_t(seconds * 1000000000.0));
+}
+
 //! Interface for architecture dependent multithreading
 /*!
 This interface defines the multithreading operations required by
@@ -98,25 +79,22 @@ public:
     //
 
     //! Create a condition variable
-    /*!
+    ArchCond newCondVar() { return new std::condition_variable; }
-    The condition variable is an opaque data type.
-    */
-    virtual ArchCond    newCondVar() = 0;
 
     //! Destroy a condition variable
-    virtual void        closeCondVar(ArchCond) = 0;
+    void closeCondVar(ArchCond cv) { delete cv; }
 
     //! Signal a condition variable
     /*!
     Signalling a condition variable releases one waiting thread.
     */
-    virtual void        signalCondVar(ArchCond) = 0;
+    void signalCondVar(ArchCond cv) { cv->notify_one(); }
 
     //! Broadcast a condition variable
     /*!
     Broadcasting a condition variable releases all waiting threads.
     */
-    virtual void        broadcastCondVar(ArchCond) = 0;
+    void broadcastCondVar(ArchCond cv) { cv->notify_all(); }
 
     //! Wait on a condition variable
     /*!
@@ -129,7 +107,7 @@ public:
 
     (Cancellation point)
     */
-    virtual bool        waitCondVar(ArchCond, ArchMutex, double timeout) = 0;
+    bool waitCondVar(ArchCond cv, ArchMutex mutex, double timeout);
 
     //
     // mutex methods
@@ -137,20 +115,22 @@ public:
 
     //! Create a recursive mutex
     /*!
-    Creates a recursive mutex.  A thread may lock a recursive mutex
+    Creates mutex.  A thread may lock a recursive mutex
     when it already holds a lock on that mutex.  The mutex is an
     opaque data type.
+
+    WARNING: this is recursive mutex on Windows and some code likely depends on it.
     */
-    virtual ArchMutex    newMutex() = 0;
+    ArchMutex newMutex() { return new std::mutex; }
 
     //! Destroy a mutex
-    virtual void        closeMutex(ArchMutex) = 0;
+    void closeMutex(ArchMutex mutex) { delete mutex; }
 
     //! Lock a mutex
-    virtual void        lockMutex(ArchMutex) = 0;
+    void lockMutex(ArchMutex mutex) { mutex->lock(); }
 
     //! Unlock a mutex
-    virtual void        unlockMutex(ArchMutex) = 0;
+    void unlockMutex(ArchMutex mutex) { mutex->unlock(); }
 
     //
     // thread methods

src/lib/arch/unix/ArchMultithreadPosix.cpp

@@ -158,162 +158,6 @@ ArchMultithreadPosix::getInstance()
     return s_instance;
 }
 
-ArchCond
-ArchMultithreadPosix::newCondVar()
-{
-    ArchCondImpl* cond = new ArchCondImpl;
-    int status = pthread_cond_init(&cond->m_cond, NULL);
-    (void)status;
-    assert(status == 0);
-    return cond;
-}
-
-void
-ArchMultithreadPosix::closeCondVar(ArchCond cond)
-{
-    int status = pthread_cond_destroy(&cond->m_cond);
-    (void)status;
-    assert(status == 0);
-    delete cond;
-}
-
-void
-ArchMultithreadPosix::signalCondVar(ArchCond cond)
-{
-    int status = pthread_cond_signal(&cond->m_cond);
-    (void)status;
-    assert(status == 0);
-}
-
-void
-ArchMultithreadPosix::broadcastCondVar(ArchCond cond)
-{
-    int status = pthread_cond_broadcast(&cond->m_cond);
-    (void)status;
-    assert(status == 0);
-}
-
-bool
-ArchMultithreadPosix::waitCondVar(ArchCond cond,
-                            ArchMutex mutex, double timeout)
-{
-    // we can't wait on a condition variable and also wake it up for
-    // cancellation since we don't use posix cancellation.  so we
-    // must wake up periodically to check for cancellation.  we
-    // can't simply go back to waiting after the check since the
-    // condition may have changed and we'll have lost the signal.
-    // so we have to return to the caller.  since the caller will
-    // always check for spurious wakeups the only drawback here is
-    // performance:  we're waking up a lot more than desired.
-    static const double maxCancellationLatency = 0.1;
-    if (timeout < 0.0 || timeout > maxCancellationLatency) {
-        timeout = maxCancellationLatency;
-    }
-
-    // see if we should cancel this thread
-    testCancelThread();
-
-    // get final time
-    struct timeval now;
-    gettimeofday(&now, NULL);
-    struct timespec finalTime;
-    finalTime.tv_sec   = now.tv_sec;
-    finalTime.tv_nsec  = now.tv_usec * 1000;
-    long timeout_sec   = (long)timeout;
-    long timeout_nsec  = (long)(1.0e+9 * (timeout - timeout_sec));
-    finalTime.tv_sec  += timeout_sec;
-    finalTime.tv_nsec += timeout_nsec;
-    if (finalTime.tv_nsec >= 1000000000) {
-        finalTime.tv_nsec -= 1000000000;
-        finalTime.tv_sec  += 1;
-    }
-
-    // wait
-    int status = pthread_cond_timedwait(&cond->m_cond,
-                            &mutex->m_mutex, &finalTime);
-
-    // check for cancel again
-    testCancelThread();
-
-    switch (status) {
-    case 0:
-        // success
-        return true;
-
-    case ETIMEDOUT:
-        return false;
-
-    default:
-        assert(0 && "condition variable wait error");
-        return false;
-    }
-}
-
-ArchMutex
-ArchMultithreadPosix::newMutex()
-{
-    pthread_mutexattr_t attr;
-    int status = pthread_mutexattr_init(&attr);
-    assert(status == 0);
-    ArchMutexImpl* mutex = new ArchMutexImpl;
-    status = pthread_mutex_init(&mutex->m_mutex, &attr);
-    assert(status == 0);
-    return mutex;
-}
-
-void
-ArchMultithreadPosix::closeMutex(ArchMutex mutex)
-{
-    int status = pthread_mutex_destroy(&mutex->m_mutex);
-    (void)status;
-    assert(status == 0);
-    delete mutex;
-}
-
-void
-ArchMultithreadPosix::lockMutex(ArchMutex mutex)
-{
-    int status = pthread_mutex_lock(&mutex->m_mutex);
-
-    switch (status) {
-    case 0:
-        // success
-        return;
-
-    case EDEADLK:
-        assert(0 && "lock already owned");
-        break;
-
-    case EAGAIN:
-        assert(0 && "too many recursive locks");
-        break;
-
-    default:
-        assert(0 && "unexpected error");
-        break;
-    }
-}
-
-void
-ArchMultithreadPosix::unlockMutex(ArchMutex mutex)
-{
-    int status = pthread_mutex_unlock(&mutex->m_mutex);
-
-    switch (status) {
-    case 0:
-        // success
-        return;
-
-    case EPERM:
-        assert(0 && "thread doesn't own a lock");
-        break;
-
-    default:
-        assert(0 && "unexpected error");
-        break;
-    }
-}
-
 ArchThread ArchMultithreadPosix::newThread(const std::function<void()>& func)
 {
     // initialize signal handler.  we do this here instead of the

src/lib/arch/unix/ArchMultithreadPosix.h

@@ -26,16 +26,6 @@
 
 #define ARCH_MULTITHREAD ArchMultithreadPosix
 
-class ArchCondImpl {
-public:
-    pthread_cond_t        m_cond;
-};
-
-class ArchMutexImpl {
-public:
-    pthread_mutex_t        m_mutex;
-};
-
 //! Posix implementation of IArchMultithread
 class ArchMultithreadPosix : public IArchMultithread {
 public:
@@ -58,15 +48,6 @@ public:
     //@}
 
     // IArchMultithread overrides
-    virtual ArchCond    newCondVar();
-    virtual void        closeCondVar(ArchCond);
-    virtual void        signalCondVar(ArchCond);
-    virtual void        broadcastCondVar(ArchCond);
-    virtual bool        waitCondVar(ArchCond, ArchMutex, double timeout);
-    virtual ArchMutex    newMutex();
-    virtual void        closeMutex(ArchMutex);
-    virtual void        lockMutex(ArchMutex);
-    virtual void        unlockMutex(ArchMutex);
     virtual ArchThread newThread(const std::function<void()>& func);
     virtual ArchThread    newCurrentThread();
     virtual ArchThread    copyThread(ArchThread);

src/lib/arch/win32/ArchMultithreadWindows.cpp

@@ -149,144 +149,6 @@ ArchMultithreadWindows::getInstance()
     return s_instance;
 }
 
-ArchCond
-ArchMultithreadWindows::newCondVar()
-{
-    ArchCondImpl* cond                       = new ArchCondImpl;
-    cond->m_events[ArchCondImpl::kSignal]    = CreateEvent(NULL,
-                                                    FALSE, FALSE, NULL);
-    cond->m_events[ArchCondImpl::kBroadcast] = CreateEvent(NULL,
-                                                    TRUE,  FALSE, NULL);
-    cond->m_waitCountMutex                    = newMutex();
-    cond->m_waitCount                         = 0;
-    return cond;
-}
-
-void
-ArchMultithreadWindows::closeCondVar(ArchCond cond)
-{
-    CloseHandle(cond->m_events[ArchCondImpl::kSignal]);
-    CloseHandle(cond->m_events[ArchCondImpl::kBroadcast]);
-    closeMutex(cond->m_waitCountMutex);
-    delete cond;
-}
-
-void
-ArchMultithreadWindows::signalCondVar(ArchCond cond)
-{
-    // is anybody waiting?
-    lockMutex(cond->m_waitCountMutex);
-    const bool hasWaiter = (cond->m_waitCount > 0);
-    unlockMutex(cond->m_waitCountMutex);
-
-    // wake one thread if anybody is waiting
-    if (hasWaiter) {
-        SetEvent(cond->m_events[ArchCondImpl::kSignal]);
-    }
-}
-
-void
-ArchMultithreadWindows::broadcastCondVar(ArchCond cond)
-{
-    // is anybody waiting?
-    lockMutex(cond->m_waitCountMutex);
-    const bool hasWaiter = (cond->m_waitCount > 0);
-    unlockMutex(cond->m_waitCountMutex);
-
-    // wake all threads if anybody is waiting
-    if (hasWaiter) {
-        SetEvent(cond->m_events[ArchCondImpl::kBroadcast]);
-    }
-}
-
-bool
-ArchMultithreadWindows::waitCondVar(ArchCond cond,
-                            ArchMutex mutex, double timeout)
-{
-    // prepare to wait
-    const DWORD winTimeout = (timeout < 0.0) ? INFINITE :
-                                static_cast<DWORD>(1000.0 * timeout);
-
-    // make a list of the condition variable events and the cancel event
-    // for the current thread.
-    HANDLE handles[4];
-    handles[0] = cond->m_events[ArchCondImpl::kSignal];
-    handles[1] = cond->m_events[ArchCondImpl::kBroadcast];
-    handles[2] = getCancelEventForCurrentThread();
-
-    // update waiter count
-    lockMutex(cond->m_waitCountMutex);
-    ++cond->m_waitCount;
-    unlockMutex(cond->m_waitCountMutex);
-
-    // release mutex.  this should be atomic with the wait so that it's
-    // impossible for another thread to signal us between the unlock and
-    // the wait, which would lead to a lost signal on broadcasts.
-    // however, we're using a manual reset event for broadcasts which
-    // stays set until we reset it, so we don't lose the broadcast.
-    unlockMutex(mutex);
-
-    // wait for a signal or broadcast
-    // TODO: this doesn't always signal when kill signals are sent
-    DWORD result = WaitForMultipleObjects(3, handles, FALSE, winTimeout);
-
-    // cancel takes priority
-    if (result != WAIT_OBJECT_0 + 2 &&
-        WaitForSingleObject(handles[2], 0) == WAIT_OBJECT_0) {
-        result = WAIT_OBJECT_0 + 2;
-    }
-
-    // update the waiter count and check if we're the last waiter
-    lockMutex(cond->m_waitCountMutex);
-    --cond->m_waitCount;
-    const bool last = (result == WAIT_OBJECT_0 + 1 && cond->m_waitCount == 0);
-    unlockMutex(cond->m_waitCountMutex);
-
-    // reset the broadcast event if we're the last waiter
-    if (last) {
-        ResetEvent(cond->m_events[ArchCondImpl::kBroadcast]);
-    }
-
-    // reacquire the mutex
-    lockMutex(mutex);
-
-    // cancel thread if necessary
-    if (result == WAIT_OBJECT_0 + 2) {
-        ARCH->testCancelThread();
-    }
-
-    // return success or failure
-    return (result == WAIT_OBJECT_0 + 0 ||
-            result == WAIT_OBJECT_0 + 1);
-}
-
-ArchMutex
-ArchMultithreadWindows::newMutex()
-{
-    ArchMutexImpl* mutex = new ArchMutexImpl;
-    InitializeCriticalSection(&mutex->m_mutex);
-    return mutex;
-}
-
-void
-ArchMultithreadWindows::closeMutex(ArchMutex mutex)
-{
-    DeleteCriticalSection(&mutex->m_mutex);
-    delete mutex;
-}
-
-void
-ArchMultithreadWindows::lockMutex(ArchMutex mutex)
-{
-    EnterCriticalSection(&mutex->m_mutex);
-}
-
-void
-ArchMultithreadWindows::unlockMutex(ArchMutex mutex)
-{
-    LeaveCriticalSection(&mutex->m_mutex);
-}
-
 ArchThread ArchMultithreadWindows::newThread(const std::function<void()>& func)
 {
     lockMutex(m_threadMutex);

src/lib/arch/win32/ArchMultithreadWindows.h

@@ -26,20 +26,6 @@
 
 #define ARCH_MULTITHREAD ArchMultithreadWindows
 
-class ArchCondImpl {
-public:
-    enum { kSignal = 0, kBroadcast };
-
-    HANDLE                m_events[2];
-    mutable int            m_waitCount;
-    ArchMutex            m_waitCountMutex;
-};
-
-class ArchMutexImpl {
-public:
-    CRITICAL_SECTION    m_mutex;
-};
-
 //! Win32 implementation of IArchMultithread
 class ArchMultithreadWindows : public IArchMultithread {
 public:
@@ -64,15 +50,6 @@ public:
     //@}
 
     // IArchMultithread overrides
-    virtual ArchCond    newCondVar();
-    virtual void        closeCondVar(ArchCond);
-    virtual void        signalCondVar(ArchCond);
-    virtual void        broadcastCondVar(ArchCond);
-    virtual bool        waitCondVar(ArchCond, ArchMutex, double timeout);
-    virtual ArchMutex    newMutex();
-    virtual void        closeMutex(ArchMutex);
-    virtual void        lockMutex(ArchMutex);
-    virtual void        unlockMutex(ArchMutex);
     virtual ArchThread newThread(const std::function<void()>& func);
     virtual ArchThread    newCurrentThread();
     virtual ArchThread    copyThread(ArchThread);

src/test/unittests/CMakeLists.txt

@@ -65,4 +65,4 @@ endif()
 
 add_executable(unittests ${sources})
 target_link_libraries(unittests
-    arch base client server common io net platform server synlib mt ipc ${GTEST_LIBRARIES} ${GMOCK_LIBRARIES} ${libs} ${OPENSSL_LIBS})
+    base client server common io net platform server synlib mt arch ipc ${GTEST_LIBRARIES} ${GMOCK_LIBRARIES} ${libs} ${OPENSSL_LIBS})