Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix C++11 Windows build of threading code
	The initial problem was that mutex_auto_lock.h tries to use std::unique_lock<std::mutex>
	despite mutex.h not using C++11's std::mutex on Windows. The problem here is the mismatch
	between C++11 usage conditions of the two headers. This commit moves the decision logic
	to threads.h and makes sure mutex.h, mutex_auto_lock.h and event.h all use the same features.
  • Loading branch information
sfan5 committed Oct 6, 2016
1 parent b66a5d2 commit 60e1d5e
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 48 deletions.
16 changes: 8 additions & 8 deletions src/threading/event.cpp
Expand Up @@ -27,8 +27,8 @@ DEALINGS IN THE SOFTWARE.

Event::Event()
{
#if __cplusplus < 201103L
# ifdef _WIN32
#ifndef USE_CPP11_MUTEX
# if USE_WIN_MUTEX
event = CreateEvent(NULL, false, false, NULL);
# else
pthread_cond_init(&cv, NULL);
Expand All @@ -38,10 +38,10 @@ Event::Event()
#endif
}

#if __cplusplus < 201103L
#ifndef USE_CPP11_MUTEX
Event::~Event()
{
#ifdef _WIN32
#if USE_WIN_MUTEX
CloseHandle(event);
#else
pthread_cond_destroy(&cv);
Expand All @@ -53,13 +53,13 @@ Event::~Event()

void Event::wait()
{
#if __cplusplus >= 201103L
#if USE_CPP11_MUTEX
MutexAutoLock lock(mutex);
while (!notified) {
cv.wait(lock);
}
notified = false;
#elif defined(_WIN32)
#elif USE_WIN_MUTEX
WaitForSingleObject(event, INFINITE);
#else
pthread_mutex_lock(&mutex);
Expand All @@ -74,11 +74,11 @@ void Event::wait()

void Event::signal()
{
#if __cplusplus >= 201103L
#if USE_CPP11_MUTEX
MutexAutoLock lock(mutex);
notified = true;
cv.notify_one();
#elif defined(_WIN32)
#elif USE_WIN_MUTEX
SetEvent(event);
#else
pthread_mutex_lock(&mutex);
Expand Down
12 changes: 7 additions & 5 deletions src/threading/event.h
Expand Up @@ -26,11 +26,13 @@ DEALINGS IN THE SOFTWARE.
#ifndef THREADING_EVENT_H
#define THREADING_EVENT_H

#if __cplusplus >= 201103L
#include "threads.h"

#if USE_CPP11_MUTEX
#include <condition_variable>
#include "threading/mutex.h"
#include "threading/mutex_auto_lock.h"
#elif defined(_WIN32)
#elif USE_WIN_MUTEX
#ifndef WIN32_LEAN_AND_MEAN
#define WIN32_LEAN_AND_MEAN
#endif
Expand All @@ -49,18 +51,18 @@ DEALINGS IN THE SOFTWARE.
class Event {
public:
Event();
#if __cplusplus < 201103L
#ifndef USE_CPP11_MUTEX
~Event();
#endif
void wait();
void signal();

private:
#if __cplusplus >= 201103L
#if USE_CPP11_MUTEX
std::condition_variable cv;
Mutex mutex;
bool notified;
#elif defined(_WIN32)
#elif USE_WIN_MUTEX
HANDLE event;
#else
pthread_cond_t cv;
Expand Down
19 changes: 9 additions & 10 deletions src/threading/mutex.cpp
Expand Up @@ -23,14 +23,13 @@ FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS IN THE SOFTWARE.
*/

// Windows std::mutex is much slower than the critical section API
#if __cplusplus < 201103L || defined(_WIN32)
#include "threads.h"

#ifndef USE_CPP11_MUTEX

#include "threading/mutex.h"

#ifndef _WIN32
#include <cassert>
#endif
#include <cassert>

#define UNUSED(expr) do { (void)(expr); } while (0)

Expand All @@ -47,7 +46,7 @@ Mutex::Mutex(bool recursive)

void Mutex::init_mutex(bool recursive)
{
#ifdef _WIN32
#if USE_WIN_MUTEX
// Windows critical sections are recursive by default
UNUSED(recursive);

Expand All @@ -69,7 +68,7 @@ void Mutex::init_mutex(bool recursive)

Mutex::~Mutex()
{
#ifdef _WIN32
#if USE_WIN_MUTEX
DeleteCriticalSection(&mutex);
#else
int ret = pthread_mutex_destroy(&mutex);
Expand All @@ -80,7 +79,7 @@ Mutex::~Mutex()

void Mutex::lock()
{
#ifdef _WIN32
#if USE_WIN_MUTEX
EnterCriticalSection(&mutex);
#else
int ret = pthread_mutex_lock(&mutex);
Expand All @@ -91,7 +90,7 @@ void Mutex::lock()

void Mutex::unlock()
{
#ifdef _WIN32
#if USE_WIN_MUTEX
LeaveCriticalSection(&mutex);
#else
int ret = pthread_mutex_unlock(&mutex);
Expand All @@ -104,5 +103,5 @@ RecursiveMutex::RecursiveMutex()
: Mutex(true)
{}

#endif
#endif // C++11

15 changes: 8 additions & 7 deletions src/threading/mutex.h
Expand Up @@ -26,22 +26,23 @@ DEALINGS IN THE SOFTWARE.
#ifndef THREADING_MUTEX_H
#define THREADING_MUTEX_H

// Windows std::mutex is much slower than the critical section API
#if __cplusplus >= 201103L && !defined(_WIN32)
#include "threads.h"

#if USE_CPP11_MUTEX
#include <mutex>
using Mutex = std::mutex;
using RecursiveMutex = std::recursive_mutex;
#else

#ifdef _WIN32
#if USE_WIN_MUTEX
#ifndef _WIN32_WINNT
#define _WIN32_WINNT 0x0501
#endif
#ifndef WIN32_LEAN_AND_MEAN
#define WIN32_LEAN_AND_MEAN
#endif
#include <windows.h>
#else // pthread
#else
#include <pthread.h>
#endif

Expand All @@ -59,9 +60,9 @@ class Mutex
Mutex(bool recursive);
void init_mutex(bool recursive);
private:
#ifdef _WIN32
#if USE_WIN_MUTEX
CRITICAL_SECTION mutex;
#else // pthread
#else
pthread_mutex_t mutex;
#endif

Expand All @@ -76,6 +77,6 @@ class RecursiveMutex : public Mutex
DISABLE_CLASS_COPY(RecursiveMutex);
};

#endif // C++11
#endif // C++11

#endif
4 changes: 3 additions & 1 deletion src/threading/mutex_auto_lock.h
Expand Up @@ -26,7 +26,9 @@ DEALINGS IN THE SOFTWARE.
#ifndef THREADING_MUTEX_AUTO_LOCK_H
#define THREADING_MUTEX_AUTO_LOCK_H

#if __cplusplus >= 201103L
#include "threads.h"

#if USE_CPP11_MUTEX
#include <mutex>
using MutexAutoLock = std::unique_lock<std::mutex>;
using RecursiveMutexAutoLock = std::unique_lock<std::recursive_mutex>;
Expand Down
20 changes: 10 additions & 10 deletions src/threading/thread.cpp
Expand Up @@ -198,7 +198,7 @@ bool Thread::kill()

m_running = false;

#ifdef _WIN32
#if USE_WIN_THREADS
TerminateThread(m_thread_handle, 0);
CloseHandle(m_thread_handle);
#else
Expand Down Expand Up @@ -310,10 +310,16 @@ void Thread::setName(const std::string &name)

unsigned int Thread::getNumberOfProcessors()
{
#if __cplusplus >= 201103L
#if USE_CPP11_THREADS

return std::thread::hardware_concurrency();

#elif USE_WIN_THREADS

SYSTEM_INFO sysinfo;
GetSystemInfo(&sysinfo);
return sysinfo.dwNumberOfProcessors;

#elif defined(_SC_NPROCESSORS_ONLN)

return sysconf(_SC_NPROCESSORS_ONLN);
Expand All @@ -335,12 +341,6 @@ unsigned int Thread::getNumberOfProcessors()

return get_nprocs();

#elif defined(_WIN32)

SYSTEM_INFO sysinfo;
GetSystemInfo(&sysinfo);
return sysinfo.dwNumberOfProcessors;

#elif defined(PTW32_VERSION) || defined(__hpux)

return pthread_num_processors_np();
Expand All @@ -359,7 +359,7 @@ bool Thread::bindToProcessor(unsigned int proc_number)

return false;

#elif defined(_WIN32)
#elif USE_WIN_THREADS

return SetThreadAffinityMask(getThreadHandle(), 1 << proc_number);

Expand Down Expand Up @@ -407,7 +407,7 @@ bool Thread::bindToProcessor(unsigned int proc_number)

bool Thread::setPriority(int prio)
{
#if defined(_WIN32)
#if USE_WIN_THREADS

return SetThreadPriority(getThreadHandle(), prio);

Expand Down
10 changes: 4 additions & 6 deletions src/threading/thread.h
Expand Up @@ -157,9 +157,11 @@ class Thread {
Atomic<bool> m_running;
Mutex m_mutex;

#ifndef USE_CPP11_THREADS
#if USE_CPP11_THREADS
std::thread *m_thread_obj;
#else
threadhandle_t m_thread_handle;
# if _WIN32
# if USE_WIN_THREADS
threadid_t m_thread_id;
# endif
#endif
Expand All @@ -172,10 +174,6 @@ class Thread {
tid_t m_kernel_thread_id;
#endif

#if USE_CPP11_THREADS
std::thread *m_thread_obj;
#endif

DISABLE_CLASS_COPY(Thread);
};

Expand Down
11 changes: 10 additions & 1 deletion src/threads.h
Expand Up @@ -21,7 +21,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
#define THREADS_HEADER

//
// Determine which threading API we will use
// Determine which threading APIs we will use
//
#if __cplusplus >= 201103L
#define USE_CPP11_THREADS 1
Expand All @@ -31,6 +31,15 @@ with this program; if not, write to the Free Software Foundation, Inc.,
#define USE_POSIX_THREADS 1
#endif

#if defined(_WIN32)
// Prefer critical section API because std::mutex is much slower on Windows
#define USE_WIN_MUTEX 1
#elif __cplusplus >= 201103L
#define USE_CPP11_MUTEX 1
#else
#define USE_POSIX_MUTEX 1
#endif

///////////////


Expand Down

0 comments on commit 60e1d5e

Please sign in to comment.