From 3243a9f8c5052d35e1a4bfccea3f88dda5ab2cdf Mon Sep 17 00:00:00 2001
From: jp9000 <obs.jim@gmail.com>
Date: Wed, 29 Jan 2014 18:27:42 -0700
Subject: [PATCH] Fix the design flaw with obs_sceneitem_destroy

The previous commit used the scene as a  parameter to check to see if
the scene item was still present within the scene before destroying, but
this was actually unnecessary because the fault was because the destroy
signal was being triggered *before* the scene's mutex locked, thus
causing a race condition.  I changed the code so that it signals after
the lock instead of before, so the scene parameter should no longer be
necessary.
---
 libobs/obs-scene.c        | 32 ++++++++++----------------------
 libobs/obs.h              |  2 +-
 obs/window-basic-main.cpp |  2 +-
 3 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/libobs/obs-scene.c b/libobs/obs-scene.c
index 630df389d..eab9298bf 100644
--- a/libobs/obs-scene.c
+++ b/libobs/obs-scene.c
@@ -124,7 +124,7 @@ static void scene_video_render(void *data)
 			struct obs_scene_item *del_item = item;
 			item = item->next;
 
-			obs_sceneitem_destroy(scene, del_item);
+			obs_sceneitem_destroy(del_item);
 			continue;
 		}
 
@@ -294,38 +294,26 @@ obs_sceneitem_t obs_scene_add(obs_scene_t scene, obs_source_t source)
 	return item;
 }
 
-int obs_sceneitem_destroy(obs_scene_t scene, obs_sceneitem_t item)
+int obs_sceneitem_destroy(obs_sceneitem_t item)
 {
 	int ref = 0;
-	if (!scene || !item)
-		return ref;
 
-	pthread_mutex_lock(&scene->mutex);
-
-	bool found = false;
-	obs_sceneitem_t i = scene->first_item;
-	while (i) {
-		if (i == item) {
-			found = true;
-			break;
-		}
-	}
-
-	if (found) {
+	if (item) {
+		obs_scene_t scene = item->parent;
 		struct calldata params = {0};
-		signal_item_destroy(item, &params);
-		calldata_free(&params);
 
-		pthread_mutex_lock(&item->parent->mutex);
+		pthread_mutex_lock(&scene->mutex);
+
+		signal_item_destroy(item, &params);
 		detach_sceneitem(item);
-		pthread_mutex_unlock(&item->parent->mutex);
 
 		if (item->source)
 			ref = obs_source_release(item->source);
 		bfree(item);
-	}
 
-	pthread_mutex_unlock(&scene->mutex);
+		pthread_mutex_unlock(&scene->mutex);
+		calldata_free(&params);
+	}
 
 	return ref;
 }
diff --git a/libobs/obs.h b/libobs/obs.h
index 535edd32d..551bcef82 100644
--- a/libobs/obs.h
+++ b/libobs/obs.h
@@ -503,7 +503,7 @@ EXPORT obs_sceneitem_t obs_scene_add(obs_scene_t scene, obs_source_t source);
 
 /** Removes/destroys a scene item.  Returns the source reference counter
  * (if any) */
-EXPORT int obs_sceneitem_destroy(obs_scene_t scene, obs_sceneitem_t item);
+EXPORT int obs_sceneitem_destroy(obs_sceneitem_t item);
 
 /** Gets the scene parent associated with the scene item */
 EXPORT obs_scene_t obs_sceneitem_getscene(obs_sceneitem_t item);
diff --git a/obs/window-basic-main.cpp b/obs/window-basic-main.cpp
index bcd25411e..ce6840e02 100644
--- a/obs/window-basic-main.cpp
+++ b/obs/window-basic-main.cpp
@@ -524,7 +524,7 @@ void OBSBasic::on_actionRemoveSource_triggered()
 
 	QVariant userData = sel->data(Qt::UserRole);
 	obs_sceneitem_t item = VariantPtr<obs_sceneitem_t>(userData);
-	obs_sceneitem_destroy(scene, item);
+	obs_sceneitem_destroy(item);
 
 	obs_scene_release(scene);
 	gs_leavecontext();