obs-scripting: Make callback "removed" variable atomic

Makes the callback variable "removed" atomic, and on script unload,
first sets all callbacks to removed before actually unloading the script
out of a safety precaution. (See note at the bottom for further details)

This minimizes the possibility of a race condition where the script
callback could be called while those callbacks were being removed.

Big note for this change, this change should eventually be replaced with
a reference counting ownership method where script callbacks can hold a
reference and share ownership of the script if it's still alive while
the script callback is being called. That way the script callbacks can
safely execute. May require a fair amount of reworking of the script
object.

deps/obs-scripting/obs-scripting-callback.h

@@ -33,9 +33,14 @@ struct script_callback {
 	obs_script_t *script;
 	calldata_t extra;
 
-	bool removed;
+	volatile bool removed;
 };
 
+static inline bool script_callback_removed(struct script_callback *cb)
+{
+	return os_atomic_load_bool(&cb->removed);
+}
+
 static inline void *add_script_callback(struct script_callback **first,
 					obs_script_t *script, size_t extra_size)
 {
@@ -54,7 +59,7 @@ static inline void *add_script_callback(struct script_callback **first,
 
 static inline void remove_script_callback(struct script_callback *cb)
 {
-	cb->removed = true;
+	os_atomic_set_bool(&cb->removed, true);
 
 	struct script_callback *next = cb->next;
 	if (next)

deps/obs-scripting/obs-scripting-lua-frontend.c

@@ -206,7 +206,7 @@ static void frontend_event_callback(enum obs_frontend_event event, void *priv)
 	struct lua_obs_callback *cb = priv;
 	lua_State *script = cb->script;
 
-	if (cb->base.removed) {
+	if (script_callback_removed(&cb->base)) {
 		obs_frontend_remove_event_callback(frontend_event_callback, cb);
 		return;
 	}
@@ -254,7 +254,7 @@ static void frontend_save_callback(obs_data_t *save_data, bool saving,
 	struct lua_obs_callback *cb = priv;
 	lua_State *script = cb->script;
 
-	if (cb->base.removed) {
+	if (script_callback_removed(&cb->base)) {
 		obs_frontend_remove_save_callback(frontend_save_callback, cb);
 		return;
 	}

deps/obs-scripting/obs-scripting-lua.c

@@ -288,7 +288,7 @@ static void timer_call(struct script_callback *p_cb)
 {
 	struct lua_obs_callback *cb = (struct lua_obs_callback *)p_cb;
 
-	if (p_cb->removed)
+	if (script_callback_removed(p_cb))
 		return;
 
 	lock_callback();
@@ -329,7 +329,7 @@ static void obs_lua_main_render_callback(void *priv, uint32_t cx, uint32_t cy)
 	struct lua_obs_callback *cb = priv;
 	lua_State *script = cb->script;
 
-	if (cb->base.removed) {
+	if (script_callback_removed(&cb->base)) {
 		obs_remove_main_render_callback(obs_lua_main_render_callback,
 						cb);
 		return;
@@ -377,7 +377,7 @@ static void obs_lua_tick_callback(void *priv, float seconds)
 	struct lua_obs_callback *cb = priv;
 	lua_State *script = cb->script;
 
-	if (cb->base.removed) {
+	if (script_callback_removed(&cb->base)) {
 		obs_remove_tick_callback(obs_lua_tick_callback, cb);
 		return;
 	}
@@ -423,7 +423,7 @@ static void calldata_signal_callback(void *priv, calldata_t *cd)
 	struct lua_obs_callback *cb = priv;
 	lua_State *script = cb->script;
 
-	if (cb->base.removed) {
+	if (script_callback_removed(&cb->base)) {
 		signal_handler_remove_current();
 		return;
 	}
@@ -505,7 +505,7 @@ static void calldata_signal_callback_global(void *priv, const char *signal,
 	struct lua_obs_callback *cb = priv;
 	lua_State *script = cb->script;
 
-	if (cb->base.removed) {
+	if (script_callback_removed(&cb->base)) {
 		signal_handler_remove_current();
 		return;
 	}
@@ -663,7 +663,7 @@ static void hotkey_pressed(void *p_cb, bool pressed)
 	struct lua_obs_callback *cb = p_cb;
 	lua_State *script = cb->script;
 
-	if (cb->base.removed)
+	if (script_callback_removed(&cb->base))
 		return;
 
 	lock_callback();
@@ -689,7 +689,7 @@ static void hotkey_callback(void *p_cb, obs_hotkey_id id, obs_hotkey_t *hotkey,
 {
 	struct lua_obs_callback *cb = p_cb;
 
-	if (cb->base.removed)
+	if (script_callback_removed(&cb->base))
 		return;
 
 	if (pressed)
@@ -745,7 +745,7 @@ static bool button_prop_clicked(obs_properties_t *props, obs_property_t *p,
 	lua_State *script = cb->script;
 	bool ret = false;
 
-	if (cb->base.removed)
+	if (script_callback_removed(&cb->base))
 		return false;
 
 	lock_callback();
@@ -801,7 +801,7 @@ static bool modified_callback(void *p_cb, obs_properties_t *props,
 	lua_State *script = cb->script;
 	bool ret = false;
 
-	if (cb->base.removed)
+	if (script_callback_removed(&cb->base))
 		return false;
 
 	lock_callback();
@@ -1078,7 +1078,7 @@ static void lua_tick(void *param, float seconds)
 		struct lua_obs_timer *next = timer->next;
 		struct lua_obs_callback *cb = lua_obs_timer_cb(timer);
 
-		if (cb->base.removed) {
+		if (script_callback_removed(&cb->base)) {
 			lua_obs_timer_remove(timer);
 		} else {
 			uint64_t elapsed = ts - timer->last_ts;
@@ -1156,6 +1156,25 @@ void obs_lua_script_unload(obs_script_t *s)
 
 	lua_State *script = data->script;
 
+	/* ---------------------------- */
+	/* mark callbacks as removed    */
+
+	pthread_mutex_lock(&data->mutex);
+
+	/* XXX: scripts can potentially make callbacks when this happens, so
+	 * this probably still isn't ideal as we can't predict how the
+	 * processor or operating system is going to schedule things. a more
+	 * ideal method would be to reference count the script objects and
+	 * atomically share ownership with callbacks when they're called. */
+	struct lua_obs_callback *cb =
+		(struct lua_obs_callback *)data->first_callback;
+	while (cb) {
+		os_atomic_set_bool(&cb->base.removed, true);
+		cb = (struct lua_obs_callback *)cb->base.next;
+	}
+
+	pthread_mutex_unlock(&data->mutex);
+
 	/* ---------------------------- */
 	/* undefine source types        */
 
@@ -1189,8 +1208,7 @@ void obs_lua_script_unload(obs_script_t *s)
 	/* ---------------------------- */
 	/* remove all callbacks         */
 
-	struct lua_obs_callback *cb =
-		(struct lua_obs_callback *)data->first_callback;
+	cb = (struct lua_obs_callback *)data->first_callback;
 	while (cb) {
 		struct lua_obs_callback *next =
 			(struct lua_obs_callback *)cb->base.next;

deps/obs-scripting/obs-scripting-python-frontend.c

@@ -280,7 +280,7 @@ static void frontend_save_callback(obs_data_t *save_data, bool saving,
 {
 	struct python_obs_callback *cb = priv;
 
-	if (cb->base.removed) {
+	if (script_callback_removed(&cb->base)) {
 		obs_frontend_remove_save_callback(frontend_save_callback, cb);
 		return;
 	}
@@ -355,7 +355,7 @@ static void frontend_event_callback(enum obs_frontend_event event, void *priv)
 {
 	struct python_obs_callback *cb = priv;
 
-	if (cb->base.removed) {
+	if (script_callback_removed(&cb->base)) {
 		obs_frontend_remove_event_callback(frontend_event_callback, cb);
 		return;
 	}

deps/obs-scripting/obs-scripting-python.c

@@ -431,7 +431,7 @@ static void timer_call(struct script_callback *p_cb)
 {
 	struct python_obs_callback *cb = (struct python_obs_callback *)p_cb;
 
-	if (p_cb->removed)
+	if (script_callback_removed(p_cb))
 		return;
 
 	lock_callback(cb);
@@ -476,7 +476,7 @@ static void obs_python_tick_callback(void *priv, float seconds)
 {
 	struct python_obs_callback *cb = priv;
 
-	if (cb->base.removed) {
+	if (script_callback_removed(&cb->base)) {
 		obs_remove_tick_callback(obs_python_tick_callback, cb);
 		return;
 	}
@@ -546,7 +546,7 @@ static void calldata_signal_callback(void *priv, calldata_t *cd)
 {
 	struct python_obs_callback *cb = priv;
 
-	if (cb->base.removed) {
+	if (script_callback_removed(&cb->base)) {
 		signal_handler_remove_current();
 		return;
 	}
@@ -652,7 +652,7 @@ static void calldata_signal_callback_global(void *priv, const char *signal,
 {
 	struct python_obs_callback *cb = priv;
 
-	if (cb->base.removed) {
+	if (script_callback_removed(&cb->base)) {
 		signal_handler_remove_current();
 		return;
 	}
@@ -767,7 +767,7 @@ static void hotkey_pressed(void *p_cb, bool pressed)
 {
 	struct python_obs_callback *cb = p_cb;
 
-	if (cb->base.removed)
+	if (script_callback_removed(&cb->base))
 		return;
 
 	lock_callback(cb);
@@ -803,7 +803,7 @@ static void hotkey_callback(void *p_cb, obs_hotkey_id id, obs_hotkey_t *hotkey,
 {
 	struct python_obs_callback *cb = p_cb;
 
-	if (cb->base.removed)
+	if (script_callback_removed(&cb->base))
 		return;
 
 	if (pressed)
@@ -873,7 +873,7 @@ static bool button_prop_clicked(obs_properties_t *props, obs_property_t *p,
 	struct python_obs_callback *cb = p_cb;
 	bool ret = false;
 
-	if (cb->base.removed)
+	if (script_callback_removed(&cb->base))
 		return false;
 
 	lock_callback(cb);
@@ -937,7 +937,7 @@ static bool modified_callback(void *p_cb, obs_properties_t *props,
 	struct python_obs_callback *cb = p_cb;
 	bool ret = false;
 
-	if (cb->base.removed)
+	if (script_callback_removed(&cb->base))
 		return false;
 
 	lock_callback(cb);
@@ -1333,6 +1333,24 @@ void obs_python_script_unload(obs_script_t *s)
 	if (!s->loaded || !python_loaded)
 		return;
 
+	/* ---------------------------- */
+	/* mark callbacks as removed    */
+
+	lock_python();
+
+	/* XXX: scripts can potentially make callbacks when this happens, so
+	 * this probably still isn't ideal as we can't predict how the
+	 * processor or operating system is going to schedule things. a more
+	 * ideal method would be to reference count the script objects and
+	 * atomically share ownership with callbacks when they're called. */
+	struct script_callback *cb = data->first_callback;
+	while (cb) {
+		os_atomic_set_bool(&cb->removed, true);
+		cb = cb->next;
+	}
+
+	unlock_python();
+
 	/* ---------------------------- */
 	/* unhook tick function         */
 
@@ -1350,7 +1368,7 @@ void obs_python_script_unload(obs_script_t *s)
 		data->next_tick = NULL;
 	}
 
-	lock_python();
+	relock_python();
 
 	Py_XDECREF(data->tick);
 	Py_XDECREF(data->save);
@@ -1364,7 +1382,7 @@ void obs_python_script_unload(obs_script_t *s)
 	/* ---------------------------- */
 	/* remove all callbacks         */
 
-	struct script_callback *cb = data->first_callback;
+	cb = data->first_callback;
 	while (cb) {
 		struct script_callback *next = cb->next;
 		remove_script_callback(cb);
@@ -1532,7 +1550,7 @@ static void python_tick(void *param, float seconds)
 		struct python_obs_timer *next = timer->next;
 		struct python_obs_callback *cb = python_obs_timer_cb(timer);
 
-		if (cb->base.removed) {
+		if (script_callback_removed(&cb->base)) {
 			python_obs_timer_remove(timer);
 		} else {
 			uint64_t elapsed = ts - timer->last_ts;

deps/obs-scripting/obs-scripting-python.h

@@ -195,6 +195,7 @@ static inline bool py_error_(const char *func, int line)
 #define py_error() py_error_(__FUNCTION__, __LINE__)
 
 #define lock_python() PyGILState_STATE gstate = PyGILState_Ensure()
+#define relock_python() gstate = PyGILState_Ensure()
 #define unlock_python() PyGILState_Release(gstate)
 
 struct py_source;