From f6c3c65e26856d0f2329e1096b6283afa6d9f4c4 Mon Sep 17 00:00:00 2001 From: WarmUpTill Date: Sat, 21 Nov 2020 01:40:24 +0100 Subject: [PATCH] resolve potential deadlocks (#66) A deadlock could occur if the save callback is called just before the frontend set functions are used. Thus unlock() is necessary before setting current scene or transition. (e.g. if start recording is activated just before using the above functions, as this apparently triggers a save() call) --- src/advanced-scene-switcher.cpp | 54 ++++++++++++++++--------- src/headers/advanced-scene-switcher.hpp | 4 +- src/headers/switcher-data-structs.hpp | 6 ++- src/switch-transitions.cpp | 24 ++++++----- 4 files changed, 55 insertions(+), 33 deletions(-) diff --git a/src/advanced-scene-switcher.cpp b/src/advanced-scene-switcher.cpp index 3e3932ae..602a043e 100644 --- a/src/advanced-scene-switcher.cpp +++ b/src/advanced-scene-switcher.cpp @@ -335,6 +335,8 @@ void SwitcherData::Thread() bool match = false; OBSWeakSource scene; OBSWeakSource transition; + bool defTransitionMatch = false; + OBSWeakSource defTransition; std::chrono::milliseconds duration; if (sleep > interval) { duration = std::chrono::milliseconds(sleep); @@ -354,8 +356,6 @@ void SwitcherData::Thread() break; } - setDefaultSceneTransitions(); - if (autoStopEnable) { autoStopStreamAndRecording(); } @@ -368,6 +368,8 @@ void SwitcherData::Thread() continue; } + checkDefaultSceneTransitions(defTransitionMatch, defTransition); + for (int switchFuncName : functionNamesByPriority) { switch (switchFuncName) { case read_file_func: @@ -422,9 +424,22 @@ void SwitcherData::Thread() if (!match && switchIfNotMatching == RANDOM_SWITCH) { checkRandom(match, scene, transition, sleep); } + + // After this point we will call frontend functions + // obs_frontend_set_current_scene() and + // obs_frontend_set_current_transition() + // + // During this time SaveSceneSwitcher() could be called + // leading to a deadlock, so we have to unlock() + lock.unlock(); + + if (!match && defTransitionMatch) { + setCurrentDefTransition(defTransition); + } + if (match) { switchScene(scene, transition, - tansitionOverrideOverride, lock); + tansitionOverrideOverride); } } endLoop: @@ -432,8 +447,7 @@ endLoop: } void switchScene(OBSWeakSource &scene, OBSWeakSource &transition, - bool &transitionOverrideOverride, - std::unique_lock &lock) + bool &transitionOverrideOverride) { obs_source_t *source = obs_weak_source_get_source(scene); obs_source_t *currentSource = obs_frontend_get_current_scene(); @@ -453,18 +467,6 @@ void switchScene(OBSWeakSource &scene, OBSWeakSource &transition, obs_source_release(source); } -bool SwitcherData::sceneChangedDuringWait() -{ - bool r = false; - obs_source_t *currentSource = obs_frontend_get_current_scene(); - if (!currentSource) - return true; - obs_source_release(currentSource); - if (waitScene && currentSource != waitScene) - r = true; - return r; -} - void SwitcherData::Start() { if (!(th && th->isRunning())) { @@ -487,6 +489,15 @@ void SwitcherData::Stop() } } +bool SwitcherData::sceneChangedDuringWait() +{ + obs_source_t *currentSource = obs_frontend_get_current_scene(); + if (!currentSource) + return true; + obs_source_release(currentSource); + return (waitScene && currentSource != waitScene); +} + /******************************************************************************** * OBS module setup ********************************************************************************/ @@ -521,7 +532,11 @@ void handleSceneChange(SwitcherData *s) //reset events only hanled on scene change s->autoStartedRecently = false; - s->changedDefTransitionRecently = false; +} + +void handleTransitionStop(SwitcherData *s) +{ + s->checkedDefTransition = false; } void setLiveTime(SwitcherData *s) @@ -546,6 +561,9 @@ static void OBSEvent(enum obs_frontend_event event, void *switcher) case OBS_FRONTEND_EVENT_SCENE_CHANGED: handleSceneChange((SwitcherData *)switcher); break; + case OBS_FRONTEND_EVENT_TRANSITION_STOPPED: + handleTransitionStop((SwitcherData *)switcher); + break; case OBS_FRONTEND_EVENT_RECORDING_STARTED: case OBS_FRONTEND_EVENT_STREAMING_STARTED: setLiveTime((SwitcherData *)switcher); diff --git a/src/headers/advanced-scene-switcher.hpp b/src/headers/advanced-scene-switcher.hpp index 0b5e8f42..d4cf89e0 100644 --- a/src/headers/advanced-scene-switcher.hpp +++ b/src/headers/advanced-scene-switcher.hpp @@ -228,10 +228,8 @@ void setNextTransition(OBSWeakSource &targetScene, obs_source_t *currentSource, void overwriteTransitionOverride(obs_weak_source_t *sceneWs, obs_source_t *transition, transitionData &td); void restoreTransitionOverride(obs_source_t *scene, transitionData td); - void switchScene(OBSWeakSource &scene, OBSWeakSource &transition, - bool &transitionOverrideOverride, - std::unique_lock &lock); + bool &transitionOverrideOverride); /******************************************************************************** * Main SwitcherData diff --git a/src/headers/switcher-data-structs.hpp b/src/headers/switcher-data-structs.hpp index 258b53f2..51dacb0a 100644 --- a/src/headers/switcher-data-structs.hpp +++ b/src/headers/switcher-data-structs.hpp @@ -99,7 +99,7 @@ struct SwitcherData { std::deque sceneTransitions; std::deque defaultSceneTransitions; - bool changedDefTransitionRecently = false; + bool checkedDefTransition = false; std::deque mediaSwitches; @@ -154,10 +154,12 @@ struct SwitcherData { bool sceneChangedDuringWait(); bool prioFuncsValid(); void writeSceneInfoToFile(); - void setDefaultSceneTransitions(); void autoStopStreamAndRecording(); void autoStartStreamRecording(); bool checkPause(); + void checkDefaultSceneTransitions(bool &match, + OBSWeakSource &transition); + void setCurrentDefTransition(OBSWeakSource &transition); void checkSceneSequence(bool &match, OBSWeakSource &scene, OBSWeakSource &transition, std::unique_lock &lock); diff --git a/src/switch-transitions.cpp b/src/switch-transitions.cpp index 23634830..778fee1e 100644 --- a/src/switch-transitions.cpp +++ b/src/switch-transitions.cpp @@ -143,9 +143,10 @@ void AdvSceneSwitcher::on_defaultTransitionsDown_clicked() switcher->defaultSceneTransitions[index + 1]); } -void SwitcherData::setDefaultSceneTransitions() +void SwitcherData::checkDefaultSceneTransitions(bool &match, + OBSWeakSource &transition) { - if (changedDefTransitionRecently) + if (checkedDefTransition) return; obs_source_t *currentSource = obs_frontend_get_current_scene(); @@ -156,22 +157,25 @@ void SwitcherData::setDefaultSceneTransitions() if (!s.initialized()) continue; - obs_source_t *transition = - obs_weak_source_get_source(s.transition); - //This might cancel the current transition - //There is no way to be sure when the previous transition finished - obs_frontend_set_current_transition(transition); + match = true; + transition = s.transition; if (verbose) s.logMatch(); - - obs_source_release(transition); break; } } obs_source_release(currentSource); obs_weak_source_release(ws); - changedDefTransitionRecently = true; + + checkedDefTransition = true; +} + +void SwitcherData::setCurrentDefTransition(OBSWeakSource &transition) +{ + obs_source_t *transitionSource = obs_weak_source_get_source(transition); + obs_frontend_set_current_transition(transitionSource); + obs_source_release(transitionSource); } void AdvSceneSwitcher::on_transitionOverridecheckBox_stateChanged(int state)