mirror/OpenRGB
1
0
Fork 0
mirror of https://github.com/CalcProgrammer1/OpenRGB.git synced 2026-07-29 14:36:06 -04:00
Code Issues Packages Projects Releases Wiki Activity

Replace thread-local SignalUpdateDepth with SignalCalls per controller. Fixes an issue where callbacks that performed an unregister or clear operation on a separate thread would still deadlock, as is the case in Visual Map Plugin. Cleaned up comments

Browse Source
This commit is contained in:
Adam Honse
2026-07-26 01:29:34 -05:00
parent ac6d89b611
commit 394a5c8eb0
2 changed files with 76 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files

139
RGBController/RGBController.cpp
View File

@@ -70,11 +70,6 @@
using namespace std::chrono_literals;
/*---------------------------------------------------------*\
| One counter per thread, shared across all controllers. |
\*---------------------------------------------------------*/
thread_local unsigned int RGBController::SignalCallDepth = 0;
RGBController::RGBController()
{
/*-----------------------------------------------------*\
@@ -90,6 +85,9 @@ RGBController::RGBController()
flags = 0;
type = DEVICE_TYPE_UNKNOWN;
SignalCalls = 0;
SignalShutdown = false;
/*-----------------------------------------------------*\
| Initialize device thread |
\*-----------------------------------------------------*/
@@ -1841,27 +1839,10 @@ void RGBController::RegisterUpdateCallback(RGBControllerCallback new_callback, v
UpdateMutex.unlock();
}
/*---------------------------------------------------------*\
| Wait until no callback call is in flight, so the |
| caller may free the callback owner once this returns. |
| Skipped when the caller is itself inside a call on |
| this thread (see SignalCallDepth). |
\*---------------------------------------------------------*/
void RGBController::WaitSignalCalls()
{
if(SignalCallDepth != 0)
{
return;
}
std::unique_lock<std::mutex> wait_lock(SignalMutex);
SignalCallsDone.wait(wait_lock, [this]{ return SignalCalls == 0; });
}
void RGBController::UnregisterUpdateCallback(void * callback_arg)
{
UpdateMutex.lock();
for(unsigned int callback_idx = 0; callback_idx < UpdateCallbackArgs.size(); callback_idx++ )
for(unsigned int callback_idx = 0; callback_idx < UpdateCallbackArgs.size(); callback_idx++)
{
if(UpdateCallbackArgs[callback_idx] == callback_arg)
{
@@ -1872,10 +1853,21 @@ void RGBController::UnregisterUpdateCallback(void * callback_arg)
UpdateMutex.unlock();
/*-----------------------------------------------------*\
| Wait for in-flight calls so the caller may free the |
| callback owner once this returns |
| If this is executing from within any controller's |
| callback, return immediately rather than waiting for |
| callback completion to avoid deadlock. |
\*-----------------------------------------------------*/
WaitSignalCalls();
if(SignalCalls != 0)
{
return;
}
/*-----------------------------------------------------*\
| Otherwise, wait for all currently active callbacks to |
| complete before returning |
\*-----------------------------------------------------*/
std::unique_lock<std::mutex> wait_lock(SignalMutex);
SignalCallsDone.wait(wait_lock, [this]{ return SignalCalls == 0; });
}
void RGBController::ClearCallbacks()
@@ -1886,62 +1878,81 @@ void RGBController::ClearCallbacks()
UpdateMutex.unlock();
/*-----------------------------------------------------*\
| Wait for in-flight calls so the caller may free the |
| callback owners once this returns |
| If this is executing from within any controller's |
| callback, return immediately rather than waiting for |
| callback completion to avoid deadlock. |
\*-----------------------------------------------------*/
WaitSignalCalls();
if(SignalCalls != 0)
{
return;
}
/*-----------------------------------------------------*\
| Otherwise, wait for all currently active callbacks to |
| complete before returning |
\*-----------------------------------------------------*/
std::unique_lock<std::mutex> wait_lock(SignalMutex);
SignalCallsDone.wait(wait_lock, [this]{ return SignalCalls == 0; });
}
void RGBController::SignalUpdate(unsigned int update_reason)
{
/*-----------------------------------------------------*\
| Copy the callback list under UpdateMutex and mark a |
| call in flight, then release before invoking so no |
| lock is held across callback code. A frozen |
| controller (shutting down) calls nothing. |
\*-----------------------------------------------------*/
std::vector<RGBControllerCallback> callbacks;
std::vector<void *> callback_args;
/*-----------------------------------------------------*\
| Lock the update mutex while obtaining a copy of the |
| callbacks, but release it before actually calling |
| them so that any call to Register/Unregister from |
| within the callback doesn't deadlock. |
\*-----------------------------------------------------*/
UpdateMutex.lock();
/*-----------------------------------------------------*\
| Lock the signal mutex and increment the signal call |
| count. Return immediately if the controller has been |
| shut down. |
\*-----------------------------------------------------*/
SignalMutex.lock();
if(SignalShutdown)
{
std::lock_guard<std::mutex> call_lock(SignalMutex);
if(SignalFrozen)
{
UpdateMutex.unlock();
return;
}
SignalCalls++;
UpdateMutex.unlock();
return;
}
SignalCalls++;
SignalMutex.unlock();
/*-----------------------------------------------------*\
| Copy the list of callbacks and unlock the update |
| mutex |
\*-----------------------------------------------------*/
callbacks = UpdateCallbacks;
callback_args = UpdateCallbackArgs;
UpdateMutex.unlock();
/*-----------------------------------------------------*\
| Invoke the copied callbacks with no lock held |
| Invoke the copied callbacks |
\*-----------------------------------------------------*/
SignalCallDepth++;
for(unsigned int callback_idx = 0; callback_idx < callbacks.size(); callback_idx++)
{
callbacks[callback_idx](callback_args[callback_idx], update_reason, this);
}
SignalCallDepth--;
/*-----------------------------------------------------*\
| Wake anyone waiting once the last call is out |
| Decrement the signal call count once callbacks have |
| been called. Notify anyone waiting on signal calls |
| to be done. |
\*-----------------------------------------------------*/
SignalMutex.lock();
SignalCalls--;
if(SignalCalls == 0)
{
std::lock_guard<std::mutex> call_lock(SignalMutex);
SignalCalls--;
if(SignalCalls == 0)
{
SignalCallsDone.notify_all();
}
SignalCallsDone.notify_all();
}
SignalMutex.unlock();
}
/*---------------------------------------------------------*\
@@ -1962,19 +1973,17 @@ void RGBController::Shutdown()
AccessMutex.lock();
/*-----------------------------------------------------*\
| Freeze the controller so no further SignalUpdate |
| calls its callbacks, then wait for any call already |
| in flight. Nothing is held across teardown, so a late |
| or cross-controller SignalUpdate returns at the |
| frozen check instead of blocking forever. |
| Shut down signal handling and unlock mutexes |
\*-----------------------------------------------------*/
UpdateMutex.lock();
{
std::lock_guard<std::mutex> call_lock(SignalMutex);
SignalFrozen = true;
}
SignalMutex.lock();
SignalShutdown = true;
SignalMutex.unlock();
UpdateMutex.unlock();
/*-----------------------------------------------------*\
| Wait for any remaining signal calls to complete |
\*-----------------------------------------------------*/
std::unique_lock<std::mutex> wait_lock(SignalMutex);
SignalCallsDone.wait(wait_lock, [this]{ return SignalCalls == 0; });
}

31
RGBController/RGBController.h
View File

@@ -320,37 +320,10 @@ private:
std::vector<RGBControllerCallback> UpdateCallbacks;
std::vector<void *> UpdateCallbackArgs;
/*-----------------------------------------------------*\
| Callback call state, guarded by SignalMutex. The |
| mutex is never held across callback code: a call |
| copies the callback list, releases, then invokes. |
| |
| SignalCalls : calls in flight for this controller. |
| SignalFrozen : set at Shutdown; no further calls run, |
| so a late SignalUpdate returns rather |
| than blocking on teardown. |
| |
| Unregister/ClearCallbacks/Shutdown remove or freeze |
| the list then wait out any call already running, so |
| on return a removed callback is not executing and |
| cannot start again, and its owner may be freed. |
\*-----------------------------------------------------*/
std::mutex SignalMutex;
std::condition_variable SignalCallsDone;
unsigned int SignalCalls = 0;
bool SignalFrozen = false;
void WaitSignalCalls();
/*-----------------------------------------------------*\
| Depth of SignalUpdate callback calls on this thread. |
| Per-thread and shared across all controllers, not |
| per-instance, so WaitSignalCalls can tell when a |
| callback is unregistering from inside its own call |
| and must not wait on itself. Not guarded by |
| SignalMutex; thread_local needs no lock. |
\*-----------------------------------------------------*/
static thread_local unsigned int SignalCallDepth;
unsigned int SignalCalls;
bool SignalShutdown;
/*-----------------------------------------------------*\
| Private path used internally by DetectionManager |