Fix TSan-reported race conditions

lib/audiodev/LinuxMidi.hpp

@@ -70,6 +70,7 @@ struct LinuxMidi : BaseAudioVoiceEngine
 
     static void MIDIReceiveProc(snd_rawmidi_t* midi, const ReceiveFunctor& receiver)
     {
+        logvisor::RegisterThreadName("Boo MIDI");
         snd_rawmidi_status_t* midiStatus;
         snd_rawmidi_status_malloc(&midiStatus);
         pthread_cleanup_push(MIDIFreeProc, midiStatus);

lib/graphicsdev/GL.cpp

@@ -1329,7 +1329,7 @@ struct GLCommandQueue : IGraphicsCommandQueue
 #if _WIN32
         std::string thrName = WCSTMBS(APP->getFriendlyName().data()) + " GL Rendering Thread";
 #else
-        std::string thrName = std::string(APP->getFriendlyName()) + " GL Rendering Thread";
+        std::string thrName = std::string(APP->getFriendlyName()) + " Render";
 #endif
         logvisor::RegisterThreadName(thrName.c_str());
         GLDataFactoryImpl* dataFactory = static_cast<GLDataFactoryImpl*>(self->m_parent->getDataFactory());

lib/inputdev/HIDListenerUdev.cpp

@@ -1,6 +1,7 @@
 #include "boo/inputdev/IHIDListener.hpp"
 #include "boo/inputdev/DeviceFinder.hpp"
 #include "boo/inputdev/HIDParser.hpp"
+#include "logvisor/logvisor.hpp"
 #include <libudev.h>
 #include <cstring>
 #include <cstdio>
@@ -30,15 +31,14 @@ class HIDListenerUdev final : public IHIDListener
     std::thread m_udevThread;
     bool m_scanningEnabled;
 
-    static void deviceConnected(HIDListenerUdev* listener,
+    void deviceConnected(udev_device* device)
-                                udev_device* device)
     {
-        if (!listener->m_scanningEnabled)
+        if (!m_scanningEnabled)
             return;
 
         /* Prevent redundant registration */
         const char* devPath = udev_device_get_syspath(device);
-        if (listener->m_finder._hasToken(devPath))
+        if (m_finder._hasToken(devPath))
             return;
 
         /* Filter to USB/BT */
@@ -140,21 +140,20 @@ class HIDListenerUdev final : public IHIDListener
         fprintf(stderr, "\n\n");
 #endif
 
-        listener->m_finder._insertToken(
+        m_finder._insertToken(std::make_unique<DeviceToken>(type, vid, pid, manuf, product, devPath));
-            std::make_unique<DeviceToken>(type, vid, pid, manuf, product, devPath));
     }
 
-    static void deviceDisconnected(HIDListenerUdev* listener,
+    void deviceDisconnected(udev_device* device)
-                                   udev_device* device)
     {
         const char* devPath = udev_device_get_syspath(device);
-        listener->m_finder._removeToken(devPath);
+        m_finder._removeToken(devPath);
     }
 
-    static void _udevProc(HIDListenerUdev* listener)
+    void _udevProc()
     {
-        udev_monitor_enable_receiving(listener->m_udevMon);
+        logvisor::RegisterThreadName("Boo udev");
-        int fd = udev_monitor_get_fd(listener->m_udevMon);
+        udev_monitor_enable_receiving(m_udevMon);
+        int fd = udev_monitor_get_fd(m_udevMon);
         while (true)
         {
             fd_set fds;
@@ -168,14 +167,14 @@ class HIDListenerUdev final : public IHIDListener
             }
             int oldtype;
             pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldtype);
-            udev_device* dev = udev_monitor_receive_device(listener->m_udevMon);
+            udev_device* dev = udev_monitor_receive_device(m_udevMon);
             if (dev)
             {
                 const char* action = udev_device_get_action(dev);
                 if (!strcmp(action, "add"))
-                    deviceConnected(listener, dev);
+                    deviceConnected(dev);
                 else if (!strcmp(action, "remove"))
-                    deviceDisconnected(listener, dev);
+                    deviceDisconnected(dev);
                 udev_device_unref(dev);
             }
             pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &oldtype);
@@ -205,7 +204,7 @@ public:
         m_scanningEnabled = false;
 
         /* Start hotplug thread */
-        m_udevThread = std::thread(_udevProc, this);
+        m_udevThread = std::thread(std::bind(&HIDListenerUdev::_udevProc, this), this);
     }
 
     ~HIDListenerUdev()
@@ -243,7 +242,7 @@ public:
             const char* devPath = udev_list_entry_get_name(uenumItem);
             udev_device* dev = udev_device_new_from_syspath(UDEV_INST, devPath);
             if (dev)
-                deviceConnected(this, dev);
+                deviceConnected(dev);
             udev_device_unref(dev);
         }
         udev_enumerate_unref(uenum);

lib/win/ApplicationWin32.cpp

@@ -441,10 +441,12 @@ public:
                     /* SetFullscreen call for OpenGL window */
                     DoSetFullscreen(*reinterpret_cast<OGLContext::Window*>(msg.wParam), msg.lParam);
                     continue;
+#if BOO_HAS_VULKAN
                 case WM_USER+6:
                     /* SetFullscreen call for Vulkan window */
                     DoSetFullscreen(*reinterpret_cast<boo::VulkanContext::Window*>(msg.wParam), msg.lParam);
                     continue;
+#endif
                 default: break;
                 }
             }

lib/win/Win32Common.hpp

@@ -57,6 +57,7 @@ static inline void SetFullscreen(OGLContext::Window& win, bool fs)
     g_nwcv.wait(lk);
 }
 
+#if BOO_HAS_VULKAN
 static inline void SetFullscreen(boo::VulkanContext::Window& win, bool fs)
 {
     std::unique_lock<std::mutex> lk(g_nwmt);
@@ -64,6 +65,7 @@ static inline void SetFullscreen(boo::VulkanContext::Window& win, bool fs)
     g_nwcv.wait(lk);
 }
 #endif
+#endif
 
 struct Boo3DAppContextWin32 : Boo3DAppContext
 {

lib/x11/ApplicationUnix.cpp

@@ -64,7 +64,7 @@ int ApplicationRun(IApplication::EPlatformType platform,
                    bool deepColor,
                    bool singleInstance)
 {
-    std::string thrName = std::string(friendlyName) + " Main Thread";
+    std::string thrName = std::string(friendlyName) + " Main";
     logvisor::RegisterThreadName(thrName.c_str());
     if (APP)
         return 1;

lib/x11/ApplicationXlib.hpp

@@ -467,7 +467,7 @@ public:
             std::unique_lock<std::mutex> innerLk(initmt);
             innerLk.unlock();
             initcv.notify_one();
-            std::string thrName = std::string(getFriendlyName()) + " Client Thread";
+            std::string thrName = std::string(getFriendlyName()) + " Client";
             logvisor::RegisterThreadName(thrName.c_str());
             clientReturn = m_callback.appMain(this);
             pthread_kill(mainThread, SIGUSR2);
@@ -574,6 +574,7 @@ public:
     
     std::shared_ptr<IWindow> newWindow(std::string_view title)
     {
+        XLockDisplay(m_xDisp);
 #if BOO_HAS_VULKAN
         std::shared_ptr<IWindow> newWindow = _WindowXlibNew(title, m_xDisp, m_xcbConn, m_xDefaultScreen, m_xIM,
                                              m_bestStyle, m_fontset, m_lastGlxCtx, (void*)m_getVkProc, &m_glContext);
@@ -582,6 +583,7 @@ public:
                                              m_bestStyle, m_fontset, m_lastGlxCtx, nullptr, &m_glContext);
 #endif
         m_windows[(Window)newWindow->getPlatformHandle()] = newWindow;
+        XUnlockDisplay(m_xDisp);
         return newWindow;
     }
 

lib/x11/WindowXlib.cpp

@@ -298,6 +298,8 @@ struct GraphicsContextXlib : IGraphicsContext
     GLContext* m_glCtx;
     Display* m_xDisp;
 
+    std::mutex m_initmt;
+    std::condition_variable m_initcv;
     std::mutex m_vsyncmt;
     std::condition_variable m_vsynccv;
 
@@ -506,15 +508,13 @@ public:
 
         /* Spawn vsync thread */
         m_vsyncRunning = true;
-        std::mutex initmt;
+        std::unique_lock<std::mutex> outerLk(m_initmt);
-        std::condition_variable initcv;
-        std::unique_lock<std::mutex> outerLk(initmt);
         m_vsyncThread = std::thread([&]()
         {
             Display* vsyncDisp;
             GLXContext vsyncCtx;
             {
-                std::unique_lock<std::mutex> innerLk(initmt);
+                std::unique_lock<std::mutex> innerLk(m_initmt);
 
                 vsyncDisp = XOpenDisplay(0);
                 if (!vsyncDisp)
@@ -531,7 +531,7 @@ public:
                 if (!glXMakeCurrent(vsyncDisp, DefaultRootWindow(vsyncDisp), vsyncCtx))
                     Log.report(logvisor::Fatal, "unable to make vsync context current");
             }
-            initcv.notify_one();
+            m_initcv.notify_one();
 
             while (m_vsyncRunning)
             {
@@ -549,7 +549,7 @@ public:
             XUnlockDisplay(vsyncDisp);
             XCloseDisplay(vsyncDisp);
         });
-        initcv.wait(outerLk);
+        m_initcv.wait(outerLk);
 
         XUnlockDisplay(m_xDisp);
         m_commandQueue = _NewGLCommandQueue(this, m_glCtx);
@@ -642,7 +642,7 @@ struct GraphicsContextXlibVulkan : GraphicsContextXlib
     std::unique_ptr<IGraphicsCommandQueue> m_commandQueue;
 
     std::thread m_vsyncThread;
-    bool m_vsyncRunning;
+    std::atomic_bool m_vsyncRunning;
 
     static void ThrowIfFailed(VkResult res)
     {
@@ -676,9 +676,9 @@ public:
             m_surface = VK_NULL_HANDLE;
         }
 
-        if (m_vsyncRunning)
+        if (m_vsyncRunning.load())
         {
-            m_vsyncRunning = false;
+            m_vsyncRunning.store(false);
             if (m_vsyncThread.joinable())
                 m_vsyncThread.join();
         }
@@ -832,16 +832,15 @@ public:
         m_ctx->initSwapChain(*m_windowCtx, m_surface, m_format, m_colorspace);
 
         /* Spawn vsync thread */
-        m_vsyncRunning = true;
+        m_vsyncRunning.store(true);
-        std::mutex initmt;
+        std::unique_lock<std::mutex> outerLk(m_initmt);
-        std::condition_variable initcv;
-        std::unique_lock<std::mutex> outerLk(initmt);
         m_vsyncThread = std::thread([&]()
         {
+            logvisor::RegisterThreadName("Boo VSync");
             Display* vsyncDisp;
             GLXContext vsyncCtx;
             {
-                std::unique_lock<std::mutex> innerLk(initmt);
+                std::unique_lock<std::mutex> innerLk(m_initmt);
 
                 vsyncDisp = XOpenDisplay(0);
                 if (!vsyncDisp)
@@ -858,9 +857,9 @@ public:
                 if (!glXMakeCurrent(vsyncDisp, DefaultRootWindow(vsyncDisp), vsyncCtx))
                     Log.report(logvisor::Fatal, "unable to make vsync context current");
             }
-            initcv.notify_one();
+            m_initcv.notify_one();
 
-            while (m_vsyncRunning)
+            while (m_vsyncRunning.load())
             {
                 unsigned int sync;
                 int err = glXWaitVideoSyncSGI(1, 0, &sync);
@@ -874,7 +873,7 @@ public:
             XUnlockDisplay(vsyncDisp);
             XCloseDisplay(vsyncDisp);
         });
-        initcv.wait(outerLk);
+        m_initcv.wait(outerLk);
 
         m_dataFactory = _NewVulkanDataFactory(this, m_ctx);
         m_commandQueue = _NewVulkanCommandQueue(m_ctx, m_ctx->m_windows[m_parentWindow].get(), this);
@@ -1118,7 +1117,9 @@ public:
     
     void setCallback(IWindowCallback* cb)
     {
+        XLockDisplay(m_xDisp);
         m_callback = cb;
+        XUnlockDisplay(m_xDisp);
     }
 
     void closeWindow()
@@ -2064,11 +2065,9 @@ std::shared_ptr<IWindow> _WindowXlibNew(std::string_view title,
                          int defaultScreen, XIM xIM, XIMStyle bestInputStyle, XFontSet fontset,
                          GLXContext lastCtx, void* vulkanHandle, GLContext* glCtx)
 {
-    XLockDisplay(display);
     std::shared_ptr<IWindow> ret = std::make_shared<WindowXlib>(title, display, xcbConn,
                                    defaultScreen, xIM, bestInputStyle, fontset, lastCtx,
                                    vulkanHandle, glCtx);
-    XUnlockDisplay(display);
     return ret;
 }
     

logvisor

@@ -1 +1 @@
-Subproject commit 2352699c6598866548b81c3886337b4a8bb91472
+Subproject commit 82f1df9c40edc99a2d68499f363161e5b0bef849