Remove workaround to avoid race conditions

src/advanced-scene-switcher.cpp

@@ -155,6 +155,7 @@ void SwitcherData::Thread()
 
 	while (true) {
 		std::unique_lock<std::mutex> lock(m);
+		mainLoopLock = &lock;
 
 		bool match = false;
 		OBSWeakSource scene;
@@ -230,19 +231,6 @@ void SwitcherData::Thread()
 
 		ClearWebsocketMessages();
 
-		// After this point we will call frontend functions like
-		// obs_frontend_set_current_scene() and
-		// obs_frontend_set_current_transition()
-		//
-		// During this time SaveSceneSwitcher() could be called
-		// leading to a deadlock with the frontend function being stuck
-		// in QMetaObject::invokeMethod() holding the mutex and
-		// OBS being stuck in SaveSceneSwitcher().
-		//
-		// So we have to unlock() risking race conditions as these are
-		// less frequent than the above described deadlock
-		lock.unlock();
-
 		if (match) {
 			if (macroMatch) {
 				runMacros();

src/macro-core/macro-action-scene-switch.cpp

@@ -11,36 +11,39 @@ bool MacroActionSwitchScene::_registered = MacroActionFactory::Register(
 	{MacroActionSwitchScene::Create, MacroActionSwitchSceneEdit::Create,
 	 "AdvSceneSwitcher.action.switchScene"});
 
-void waitForTransitionChange(OBSWeakSource &transition)
+static void waitForTransitionChange(OBSWeakSource &transition,
+				    std::unique_lock<std::mutex> *lock,
+				    Macro *macro)
 {
 	const auto time = 100ms;
 	obs_source_t *source = obs_weak_source_get_source(transition);
-	std::unique_lock<std::mutex> lock(switcher->m);
+
 	bool stillTransitioning = true;
-	while (stillTransitioning && !switcher->abortMacroWait) {
+	while (stillTransitioning && !switcher->abortMacroWait &&
-		switcher->macroTransitionCv.wait_for(lock, time);
+	       !macro->GetStop()) {
+		switcher->macroTransitionCv.wait_for(*lock, time);
 		float t = obs_transition_get_time(source);
 		stillTransitioning = t < 1.0f && t > 0.0f;
 	}
 	obs_source_release(source);
 }
 
-void waitForTransitionChangeFixedDuration(int duration)
+static void waitForTransitionChangeFixedDuration(
+	int duration, std::unique_lock<std::mutex> *lock, Macro *macro)
 {
 	duration += 200; // It seems to be necessary to add a small buffer
 	auto time = std::chrono::high_resolution_clock::now() +
 		    std::chrono::milliseconds(duration);
 
-	std::unique_lock<std::mutex> lock(switcher->m);
+	while (!switcher->abortMacroWait && !macro->GetStop()) {
-	while (!switcher->abortMacroWait) {
+		if (switcher->macroTransitionCv.wait_until(*lock, time) ==
-		if (switcher->macroTransitionCv.wait_until(lock, time) ==
 		    std::cv_status::timeout) {
 			break;
 		}
 	}
 }
 
-int getTransitionOverrideDuration(OBSWeakSource &scene)
+static int getTransitionOverrideDuration(OBSWeakSource &scene)
 {
 	int duration = 0;
 	obs_source_t *source = obs_weak_source_get_source(scene);
@@ -54,7 +57,7 @@ int getTransitionOverrideDuration(OBSWeakSource &scene)
 	return duration;
 }
 
-bool isUsingFixedLengthTransition(const OBSWeakSource &transition)
+static bool isUsingFixedLengthTransition(const OBSWeakSource &transition)
 {
 	obs_source_t *source = obs_weak_source_get_source(transition);
 	bool ret = obs_transition_fixed(source);
@@ -62,7 +65,7 @@ bool isUsingFixedLengthTransition(const OBSWeakSource &transition)
 	return ret;
 }
 
-OBSWeakSource getOverrideTransition(OBSWeakSource &scene)
+static OBSWeakSource getOverrideTransition(OBSWeakSource &scene)
 {
 	OBSWeakSource transition;
 	obs_source_t *source = obs_weak_source_get_source(scene);
@@ -74,8 +77,8 @@ OBSWeakSource getOverrideTransition(OBSWeakSource &scene)
 	return transition;
 }
 
-int getExpectedTransitionDuration(OBSWeakSource &scene, OBSWeakSource &t,
+static int getExpectedTransitionDuration(OBSWeakSource &scene, OBSWeakSource &t,
-				  double duration)
+					 double duration)
 {
 	OBSWeakSource transition = t;
 	if (!switcher->transitionOverrideOverride) {
@@ -96,6 +99,37 @@ int getExpectedTransitionDuration(OBSWeakSource &scene, OBSWeakSource &t,
 	return obs_frontend_get_transition_duration();
 }
 
+bool MacroActionSwitchScene::WaitForTransition(OBSWeakSource &scene,
+					       OBSWeakSource &transition)
+{
+	const int expectedTransitionDuration = getExpectedTransitionDuration(
+		scene, transition, _duration.seconds);
+	switcher->abortMacroWait = false;
+
+	bool isInMainLoop = QThread::currentThread() == switcher->th;
+	if (isInMainLoop) {
+		if (expectedTransitionDuration < 0) {
+			waitForTransitionChange(transition, switcher->GetLock(),
+						GetMacro());
+		} else {
+			waitForTransitionChangeFixedDuration(
+				expectedTransitionDuration, switcher->GetLock(),
+				GetMacro());
+		}
+	} else {
+		std::mutex temp;
+		std::unique_lock<std::mutex> lock(temp);
+		if (expectedTransitionDuration < 0) {
+			waitForTransitionChange(transition, &lock, GetMacro());
+		} else {
+			waitForTransitionChangeFixedDuration(
+				expectedTransitionDuration, &lock, GetMacro());
+		}
+	}
+
+	return !switcher->abortMacroWait;
+}
+
 bool MacroActionSwitchScene::PerformAction()
 {
 	auto scene = _scene.GetScene();
@@ -103,17 +137,7 @@ bool MacroActionSwitchScene::PerformAction()
 	switchScene({scene, transition, (int)(_duration.seconds * 1000)},
 		    obs_frontend_preview_program_mode_active());
 	if (_blockUntilTransitionDone && scene) {
-		const int expectedTransitionDuration =
+		return WaitForTransition(scene, transition);
-			getExpectedTransitionDuration(scene, transition,
-						      _duration.seconds);
-		switcher->abortMacroWait = false;
-		if (expectedTransitionDuration < 0) {
-			waitForTransitionChange(transition);
-		} else {
-			waitForTransitionChangeFixedDuration(
-				expectedTransitionDuration);
-		}
-		return !switcher->abortMacroWait;
 	}
 	return true;
 }

src/macro-core/macro-action-scene-switch.hpp

@@ -26,7 +26,7 @@ public:
 	bool _blockUntilTransitionDone = true;
 
 private:
-	const char *getType() { return "MacroActionSwitchScene"; }
+	bool WaitForTransition(OBSWeakSource &scene, OBSWeakSource &transition);
 
 	static bool _registered;
 	static const std::string id;

src/macro-core/macro-action-wait.cpp

@@ -19,6 +19,17 @@ static std::map<WaitType, std::string> waitTypes = {
 static std::random_device rd;
 static std::default_random_engine re(rd());
 
+static void waitHelper(std::unique_lock<std::mutex> *lock, Macro *macro,
+		       std::chrono::high_resolution_clock::time_point &time)
+{
+	while (!switcher->abortMacroWait && !macro->GetStop()) {
+		if (switcher->macroWaitCv.wait_until(*lock, time) ==
+		    std::cv_status::timeout) {
+			break;
+		}
+	}
+}
+
 bool MacroActionWait::PerformAction()
 {
 	double sleepDuration;
@@ -39,14 +50,15 @@ bool MacroActionWait::PerformAction()
 
 	auto time = std::chrono::high_resolution_clock::now() +
 		    std::chrono::milliseconds((int)(sleepDuration * 1000));
-	auto macro = GetMacro();
+
 	switcher->abortMacroWait = false;
-	std::unique_lock<std::mutex> lock(switcher->m);
+	bool isInMainLoop = QThread::currentThread() == switcher->th;
-	while (!switcher->abortMacroWait && !macro->GetStop()) {
+	if (isInMainLoop) {
-		if (switcher->macroWaitCv.wait_until(lock, time) ==
+		waitHelper(switcher->GetLock(), GetMacro(), time);
-		    std::cv_status::timeout) {
+	} else {
-			break;
+		std::mutex temp;
-		}
+		std::unique_lock<std::mutex> lock(temp);
+		waitHelper(&lock, GetMacro(), time);
 	}
 
 	return !switcher->abortMacroWait;

src/macro-core/macro.cpp

@@ -741,9 +741,6 @@ bool SwitcherData::checkMacros()
 
 bool SwitcherData::runMacros()
 {
-	// TODO: Don't rely on creating a copy of each macro once new frontend
-	// events are available - see:
-	// https://github.com/obsproject/obs-studio/commit/feda1aaa283e8a99f6ba1159cfe6b9c1f2934a61
 	for (auto m : macros) {
 		if (m->Matched()) {
 			vblog(LOG_INFO, "running macro: %s", m->Name().c_str());

src/switcher-data-structs.hpp

@@ -90,6 +90,8 @@ struct SwitcherData {
 
 	std::condition_variable cv;
 	std::mutex m;
+	std::unique_lock<std::mutex> *mainLoopLock;
+
 	bool transitionActive = false;
 	bool waitForTransition = false;
 	bool stop = false;
@@ -251,6 +253,7 @@ struct SwitcherData {
 	void Thread();
 	void Start();
 	void Stop();
+	std::unique_lock<std::mutex> *GetLock() { return mainLoopLock; }
 
 	void setWaitScene();
 	bool sceneChangedDuringWait();