Browse Source

Fix deadlock in MIDI Map dataFromJson() and onReset() by creating a few Engine::*_NoLock() methods and calling them. Make Engine::updateParamHandle() write-lock instead of read-lock.

tags/v2.0.0
Andrew Belt 3 years ago
parent
commit
0fd6d8a858
3 changed files with 24 additions and 8 deletions
  1. +4
    -1
      include/engine/Engine.hpp
  2. +2
    -2
      src/core/MIDIMap.cpp
  3. +18
    -5
      src/engine/Engine.cpp

+ 4
- 1
include/engine/Engine.hpp View File

@@ -125,6 +125,7 @@ struct Engine {
Read-locks. Read-locks.
*/ */
Module* getModule(int64_t moduleId); Module* getModule(int64_t moduleId);
Module* getModule_NoLock(int64_t moduleId);
/** Triggers a ResetEvent for the given Module. /** Triggers a ResetEvent for the given Module.
Write-locks. Write-locks.
*/ */
@@ -208,15 +209,17 @@ struct Engine {
Read-locks. Read-locks.
*/ */
ParamHandle* getParamHandle(int64_t moduleId, int paramId); ParamHandle* getParamHandle(int64_t moduleId, int paramId);
ParamHandle* getParamHandle_NoLock(int64_t moduleId, int paramId);
/** Use getParamHandle(moduleId, paramId) instead. /** Use getParamHandle(moduleId, paramId) instead.
Read-locks. Read-locks.
*/ */
DEPRECATED ParamHandle* getParamHandle(Module* module, int paramId); DEPRECATED ParamHandle* getParamHandle(Module* module, int paramId);
/** Sets the ParamHandle IDs and module pointer. /** Sets the ParamHandle IDs and module pointer.
If `overwrite` is true and another ParamHandle points to the same param, unsets that one and replaces it with the given handle. If `overwrite` is true and another ParamHandle points to the same param, unsets that one and replaces it with the given handle.
Read-locks.
Write-locks.
*/ */
void updateParamHandle(ParamHandle* paramHandle, int64_t moduleId, int paramId, bool overwrite = true); void updateParamHandle(ParamHandle* paramHandle, int64_t moduleId, int paramId, bool overwrite = true);
void updateParamHandle_NoLock(ParamHandle* paramHandle, int64_t moduleId, int paramId, bool overwrite = true);


/** Serializes the rack. /** Serializes the rack.
Read-locks. Read-locks.


+ 2
- 2
src/core/MIDIMap.cpp View File

@@ -159,7 +159,7 @@ struct MIDIMap : Module {
void clearMap(int id) { void clearMap(int id) {
learningId = -1; learningId = -1;
ccs[id] = -1; ccs[id] = -1;
APP->engine->updateParamHandle(&paramHandles[id], -1, 0, true);
APP->engine->updateParamHandle_NoLock(&paramHandles[id], -1, 0, true);
valueFilters[id].reset(); valueFilters[id].reset();
updateMapLen(); updateMapLen();
refreshParamHandleText(id); refreshParamHandleText(id);
@@ -169,7 +169,7 @@ struct MIDIMap : Module {
learningId = -1; learningId = -1;
for (int id = 0; id < MAX_CHANNELS; id++) { for (int id = 0; id < MAX_CHANNELS; id++) {
ccs[id] = -1; ccs[id] = -1;
APP->engine->updateParamHandle(&paramHandles[id], -1, 0, true);
APP->engine->updateParamHandle_NoLock(&paramHandles[id], -1, 0, true);
valueFilters[id].reset(); valueFilters[id].reset();
refreshParamHandleText(id); refreshParamHandleText(id);
} }


+ 18
- 5
src/engine/Engine.cpp View File

@@ -878,6 +878,11 @@ bool Engine::hasModule(Module* module) {


Module* Engine::getModule(int64_t moduleId) { Module* Engine::getModule(int64_t moduleId) {
ReadLock lock(internal->mutex); ReadLock lock(internal->mutex);
return getModule_NoLock(moduleId);
}


Module* Engine::getModule_NoLock(int64_t moduleId) {
auto it = internal->modulesCache.find(moduleId); auto it = internal->modulesCache.find(moduleId);
if (it == internal->modulesCache.end()) if (it == internal->modulesCache.end())
return NULL; return NULL;
@@ -1153,6 +1158,11 @@ void Engine::removeParamHandle_NoLock(ParamHandle* paramHandle) {


ParamHandle* Engine::getParamHandle(int64_t moduleId, int paramId) { ParamHandle* Engine::getParamHandle(int64_t moduleId, int paramId) {
ReadLock lock(internal->mutex); ReadLock lock(internal->mutex);
return getParamHandle_NoLock(moduleId, paramId);
}


ParamHandle* Engine::getParamHandle_NoLock(int64_t moduleId, int paramId) {
auto it = internal->paramHandlesCache.find(std::make_tuple(moduleId, paramId)); auto it = internal->paramHandlesCache.find(std::make_tuple(moduleId, paramId));
if (it == internal->paramHandlesCache.end()) if (it == internal->paramHandlesCache.end())
return NULL; return NULL;
@@ -1166,7 +1176,12 @@ ParamHandle* Engine::getParamHandle(Module* module, int paramId) {




void Engine::updateParamHandle(ParamHandle* paramHandle, int64_t moduleId, int paramId, bool overwrite) { void Engine::updateParamHandle(ParamHandle* paramHandle, int64_t moduleId, int paramId, bool overwrite) {
ReadLock lock(internal->mutex);
WriteLock lock(internal->mutex);
updateParamHandle_NoLock(paramHandle, moduleId, paramId, overwrite);
}


void Engine::updateParamHandle_NoLock(ParamHandle* paramHandle, int64_t moduleId, int paramId, bool overwrite) {
// Check that it exists // Check that it exists
auto it = internal->paramHandles.find(paramHandle); auto it = internal->paramHandles.find(paramHandle);
assert(it != internal->paramHandles.end()); assert(it != internal->paramHandles.end());
@@ -1179,8 +1194,7 @@ void Engine::updateParamHandle(ParamHandle* paramHandle, int64_t moduleId, int p


if (paramHandle->moduleId >= 0) { if (paramHandle->moduleId >= 0) {
// Replace old ParamHandle, or reset the current ParamHandle // Replace old ParamHandle, or reset the current ParamHandle
// TODO Maybe call getParamHandle_NoLock()?
ParamHandle* oldParamHandle = getParamHandle(moduleId, paramId);
ParamHandle* oldParamHandle = getParamHandle_NoLock(moduleId, paramId);
if (oldParamHandle) { if (oldParamHandle) {
if (overwrite) { if (overwrite) {
oldParamHandle->moduleId = -1; oldParamHandle->moduleId = -1;
@@ -1197,8 +1211,7 @@ void Engine::updateParamHandle(ParamHandle* paramHandle, int64_t moduleId, int p


// Set module pointer if the above block didn't reset it // Set module pointer if the above block didn't reset it
if (paramHandle->moduleId >= 0) { if (paramHandle->moduleId >= 0) {
// TODO Maybe call getModule_NoLock()?
paramHandle->module = getModule(paramHandle->moduleId);
paramHandle->module = getModule_NoLock(paramHandle->moduleId);
} }


Engine_refreshParamHandleCache(this); Engine_refreshParamHandleCache(this);


Loading…
Cancel
Save