From 05f4e7360886e36b221ef5c3af4426625a3de686 Mon Sep 17 00:00:00 2001 From: Michael Lutz Date: Mon, 11 Mar 2019 00:45:39 +0100 Subject: Codechange: Replace custom mutex code with C++11 mutex'es. A conforming compiler with a valid -header is expected. Most parts of the code assume that locking a mutex will never fail unexpectedly, which is generally true on all common platforms that don't just pretend to be C++11. The use of condition variables in driver code is checked. --- src/video/sdl_v.cpp | 62 +++++++++++++++++++++--------------- src/video/win32_v.cpp | 87 +++++++++++++++++++++++++++++---------------------- 2 files changed, 87 insertions(+), 62 deletions(-) (limited to 'src/video') diff --git a/src/video/sdl_v.cpp b/src/video/sdl_v.cpp index 62bbb3301..b1609f7b3 100644 --- a/src/video/sdl_v.cpp +++ b/src/video/sdl_v.cpp @@ -25,6 +25,8 @@ #include "../framerate_type.h" #include "sdl_v.h" #include +#include +#include #include "../safeguards.h" @@ -39,7 +41,9 @@ static bool _draw_threaded; /** Thread used to 'draw' to the screen, i.e. push data to the screen. */ static ThreadObject *_draw_thread = NULL; /** Mutex to keep the access to the shared memory controlled. */ -static ThreadMutex *_draw_mutex = NULL; +static std::recursive_mutex *_draw_mutex = NULL; +/** Signal to draw the next frame. */ +static std::condition_variable_any *_draw_signal = NULL; /** Should we keep continue drawing? */ static volatile bool _draw_continue; static Palette _local_palette; @@ -172,20 +176,19 @@ static void DrawSurfaceToScreen() static void DrawSurfaceToScreenThread(void *) { /* First tell the main thread we're started */ - _draw_mutex->BeginCritical(); - _draw_mutex->SendSignal(); + std::unique_lock lock(*_draw_mutex); + _draw_signal->notify_one(); /* Now wait for the first thing to draw! */ - _draw_mutex->WaitForSignal(); + _draw_signal->wait(*_draw_mutex); while (_draw_continue) { CheckPaletteAnim(); /* Then just draw and wait till we stop */ DrawSurfaceToScreen(); - _draw_mutex->WaitForSignal(); + _draw_signal->wait(lock); } - _draw_mutex->EndCritical(); _draw_thread->Exit(); } @@ -668,26 +671,31 @@ void VideoDriver_SDL::MainLoop() CheckPaletteAnim(); + std::unique_lock draw_lock; if (_draw_threaded) { /* Initialise the mutex first, because that's the thing we *need* * directly in the newly created thread. */ - _draw_mutex = ThreadMutex::New(); + _draw_mutex = new std::recursive_mutex(); if (_draw_mutex == NULL) { _draw_threaded = false; } else { - _draw_mutex->BeginCritical(); + draw_lock = std::unique_lock(*_draw_mutex); + _draw_signal = new std::condition_variable_any(); _draw_continue = true; _draw_threaded = ThreadObject::New(&DrawSurfaceToScreenThread, NULL, &_draw_thread, "ottd:draw-sdl"); /* Free the mutex if we won't be able to use it. */ if (!_draw_threaded) { - _draw_mutex->EndCritical(); + draw_lock.unlock(); + draw_lock.release(); delete _draw_mutex; + delete _draw_signal; _draw_mutex = NULL; + _draw_signal = NULL; } else { /* Wait till the draw mutex has started itself. */ - _draw_mutex->WaitForSignal(); + _draw_signal->wait(*_draw_mutex); } } } @@ -752,19 +760,19 @@ void VideoDriver_SDL::MainLoop() /* The gameloop is the part that can run asynchronously. The rest * except sleeping can't. */ - if (_draw_mutex != NULL) _draw_mutex->EndCritical(); + if (_draw_mutex != NULL) draw_lock.unlock(); GameLoop(); - if (_draw_mutex != NULL) _draw_mutex->BeginCritical(); + if (_draw_mutex != NULL) draw_lock.lock(); UpdateWindows(); _local_palette = _cur_palette; } else { /* Release the thread while sleeping */ - if (_draw_mutex != NULL) _draw_mutex->EndCritical(); + if (_draw_mutex != NULL) draw_lock.unlock(); CSleep(1); - if (_draw_mutex != NULL) _draw_mutex->BeginCritical(); + if (_draw_mutex != NULL) draw_lock.lock(); NetworkDrawChatMessage(); DrawMouseCursor(); @@ -772,7 +780,7 @@ void VideoDriver_SDL::MainLoop() /* End of the critical part. */ if (_draw_mutex != NULL && !HasModalProgress()) { - _draw_mutex->SendSignal(); + _draw_signal->notify_one(); } else { /* Oh, we didn't have threads, then just draw unthreaded */ CheckPaletteAnim(); @@ -784,29 +792,34 @@ void VideoDriver_SDL::MainLoop() _draw_continue = false; /* Sending signal if there is no thread blocked * is very valid and results in noop */ - _draw_mutex->SendSignal(); - _draw_mutex->EndCritical(); + _draw_signal->notify_one(); + if (draw_lock.owns_lock()) draw_lock.unlock(); + draw_lock.release(); _draw_thread->Join(); delete _draw_mutex; + delete _draw_signal; delete _draw_thread; _draw_mutex = NULL; + _draw_signal = NULL; _draw_thread = NULL; } } bool VideoDriver_SDL::ChangeResolution(int w, int h) { - if (_draw_mutex != NULL) _draw_mutex->BeginCritical(true); - bool ret = CreateMainSurface(w, h); - if (_draw_mutex != NULL) _draw_mutex->EndCritical(true); - return ret; + std::unique_lock lock; + if (_draw_mutex != NULL) lock = std::unique_lock(*_draw_mutex); + + return CreateMainSurface(w, h); } bool VideoDriver_SDL::ToggleFullscreen(bool fullscreen) { - if (_draw_mutex != NULL) _draw_mutex->BeginCritical(true); + std::unique_lock lock; + if (_draw_mutex != NULL) lock = std::unique_lock(*_draw_mutex); + _fullscreen = fullscreen; GetVideoModes(); // get the list of available video modes bool ret = _num_resolutions != 0 && CreateMainSurface(_cur_resolution.width, _cur_resolution.height); @@ -816,7 +829,6 @@ bool VideoDriver_SDL::ToggleFullscreen(bool fullscreen) _fullscreen ^= true; } - if (_draw_mutex != NULL) _draw_mutex->EndCritical(true); return ret; } @@ -827,12 +839,12 @@ bool VideoDriver_SDL::AfterBlitterChange() void VideoDriver_SDL::AcquireBlitterLock() { - if (_draw_mutex != NULL) _draw_mutex->BeginCritical(true); + if (_draw_mutex != NULL) _draw_mutex->lock(); } void VideoDriver_SDL::ReleaseBlitterLock() { - if (_draw_mutex != NULL) _draw_mutex->EndCritical(true); + if (_draw_mutex != NULL) _draw_mutex->unlock(); } #endif /* WITH_SDL */ diff --git a/src/video/win32_v.cpp b/src/video/win32_v.cpp index 6cee4fef2..ef7bc89e2 100644 --- a/src/video/win32_v.cpp +++ b/src/video/win32_v.cpp @@ -27,6 +27,8 @@ #include "win32_v.h" #include #include +#include +#include #include "../safeguards.h" @@ -68,9 +70,9 @@ static bool _draw_threaded; /** Thread used to 'draw' to the screen, i.e. push data to the screen. */ static ThreadObject *_draw_thread = NULL; /** Mutex to keep the access to the shared memory controlled. */ -static ThreadMutex *_draw_mutex = NULL; -/** Event that is signaled when the drawing thread has finished initializing. */ -static HANDLE _draw_thread_initialized = NULL; +static std::recursive_mutex *_draw_mutex = NULL; +/** Signal to draw the next frame. */ +static std::condition_variable_any *_draw_signal = NULL; /** Should we keep continue drawing? */ static volatile bool _draw_continue; /** Local copy of the palette for use in the drawing thread. */ @@ -396,11 +398,11 @@ static void PaintWindow(HDC dc) static void PaintWindowThread(void *) { /* First tell the main thread we're started */ - _draw_mutex->BeginCritical(); - SetEvent(_draw_thread_initialized); + std::unique_lock lock(*_draw_mutex); + _draw_signal->notify_one(); /* Now wait for the first thing to draw! */ - _draw_mutex->WaitForSignal(); + _draw_signal->wait(*_draw_mutex); while (_draw_continue) { /* Convert update region from logical to device coordinates. */ @@ -422,10 +424,9 @@ static void PaintWindowThread(void *) /* Flush GDI buffer to ensure drawing here doesn't conflict with any GDI usage in the main WndProc. */ GdiFlush(); - _draw_mutex->WaitForSignal(); + _draw_signal->wait(*_draw_mutex); } - _draw_mutex->EndCritical(); _draw_thread->Exit(); } @@ -658,7 +659,7 @@ static LRESULT CALLBACK WndProcGdi(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lP /* Mark the window as updated, otherwise Windows would send more WM_PAINT messages. */ ValidateRect(hwnd, NULL); - _draw_mutex->SendSignal(); + _draw_signal->notify_one(); } else { PAINTSTRUCT ps; @@ -1189,28 +1190,36 @@ void VideoDriver_Win32::MainLoop() uint32 last_cur_ticks = cur_ticks; uint32 next_tick = cur_ticks + MILLISECONDS_PER_TICK; + std::unique_lock draw_lock; + if (_draw_threaded) { /* Initialise the mutex first, because that's the thing we *need* * directly in the newly created thread. */ - _draw_mutex = ThreadMutex::New(); - _draw_thread_initialized = CreateEvent(NULL, FALSE, FALSE, NULL); - if (_draw_mutex == NULL || _draw_thread_initialized == NULL) { + try { + _draw_signal = new std::condition_variable_any(); + _draw_mutex = new std::recursive_mutex(); + } catch (...) { _draw_threaded = false; - } else { + } + + if (_draw_threaded) { + draw_lock = std::unique_lock(*_draw_mutex); + _draw_continue = true; _draw_threaded = ThreadObject::New(&PaintWindowThread, NULL, &_draw_thread, "ottd:draw-win32"); /* Free the mutex if we won't be able to use it. */ if (!_draw_threaded) { + draw_lock.unlock(); + draw_lock.release(); delete _draw_mutex; + delete _draw_signal; _draw_mutex = NULL; - CloseHandle(_draw_thread_initialized); - _draw_thread_initialized = NULL; + _draw_signal = NULL; } else { DEBUG(driver, 1, "Threaded drawing enabled"); /* Wait till the draw thread has started itself. */ - WaitForSingleObject(_draw_thread_initialized, INFINITE); - _draw_mutex->BeginCritical(); + _draw_signal->wait(*_draw_mutex); } } } @@ -1227,7 +1236,7 @@ void VideoDriver_Win32::MainLoop() if (EditBoxInGlobalFocus()) TranslateMessage(&mesg); DispatchMessage(&mesg); } - if (_exit_game) return; + if (_exit_game) break; #if defined(_DEBUG) if (_wnd.has_focus && GetAsyncKeyState(VK_SHIFT) < 0 && @@ -1270,9 +1279,9 @@ void VideoDriver_Win32::MainLoop() /* The game loop is the part that can run asynchronously. * The rest except sleeping can't. */ - if (_draw_threaded) _draw_mutex->EndCritical(); + if (_draw_threaded) draw_lock.unlock(); GameLoop(); - if (_draw_threaded) _draw_mutex->BeginCritical(); + if (_draw_threaded) draw_lock.lock(); if (_force_full_redraw) MarkWholeScreenDirty(); @@ -1283,9 +1292,9 @@ void VideoDriver_Win32::MainLoop() GdiFlush(); /* Release the thread while sleeping */ - if (_draw_threaded) _draw_mutex->EndCritical(); + if (_draw_threaded) draw_lock.unlock(); Sleep(1); - if (_draw_threaded) _draw_mutex->BeginCritical(); + if (_draw_threaded) draw_lock.lock(); NetworkDrawChatMessage(); DrawMouseCursor(); @@ -1296,35 +1305,38 @@ void VideoDriver_Win32::MainLoop() _draw_continue = false; /* Sending signal if there is no thread blocked * is very valid and results in noop */ - _draw_mutex->SendSignal(); - _draw_mutex->EndCritical(); + _draw_signal->notify_all(); + if (draw_lock.owns_lock()) draw_lock.unlock(); + draw_lock.release(); _draw_thread->Join(); - CloseHandle(_draw_thread_initialized); delete _draw_mutex; + delete _draw_signal; delete _draw_thread; + + _draw_mutex = NULL; } } bool VideoDriver_Win32::ChangeResolution(int w, int h) { - if (_draw_mutex != NULL) _draw_mutex->BeginCritical(true); + std::unique_lock lock; + if (_draw_mutex != NULL) lock = std::unique_lock(*_draw_mutex); + if (_window_maximize) ShowWindow(_wnd.main_wnd, SW_SHOWNORMAL); _wnd.width = _wnd.width_org = w; _wnd.height = _wnd.height_org = h; - bool ret = this->MakeWindow(_fullscreen); // _wnd.fullscreen screws up ingame resolution switching - if (_draw_mutex != NULL) _draw_mutex->EndCritical(true); - return ret; + return this->MakeWindow(_fullscreen); // _wnd.fullscreen screws up ingame resolution switching } bool VideoDriver_Win32::ToggleFullscreen(bool full_screen) { - if (_draw_mutex != NULL) _draw_mutex->BeginCritical(true); - bool ret = this->MakeWindow(full_screen); - if (_draw_mutex != NULL) _draw_mutex->EndCritical(true); - return ret; + std::unique_lock lock; + if (_draw_mutex != NULL) lock = std::unique_lock(*_draw_mutex); + + return this->MakeWindow(full_screen); } bool VideoDriver_Win32::AfterBlitterChange() @@ -1334,19 +1346,20 @@ bool VideoDriver_Win32::AfterBlitterChange() void VideoDriver_Win32::AcquireBlitterLock() { - if (_draw_mutex != NULL) _draw_mutex->BeginCritical(true); + if (_draw_mutex != NULL) _draw_mutex->lock(); } void VideoDriver_Win32::ReleaseBlitterLock() { - if (_draw_mutex != NULL) _draw_mutex->EndCritical(true); + if (_draw_mutex != NULL) _draw_mutex->unlock(); } void VideoDriver_Win32::EditBoxLostFocus() { - if (_draw_mutex != NULL) _draw_mutex->BeginCritical(true); + std::unique_lock lock; + if (_draw_mutex != NULL) lock = std::unique_lock(*_draw_mutex); + CancelIMEComposition(_wnd.main_wnd); SetCompositionPos(_wnd.main_wnd); SetCandidatePos(_wnd.main_wnd); - if (_draw_mutex != NULL) _draw_mutex->EndCritical(true); } -- cgit v1.2.3-54-g00ecf