Skip to content

Commit 0757185

Browse files
committed
player/scripting: fix race condition when destroying clients
Clients run in detached threads and are waited to signal when they are finished with self mpv_destroy() call. Problem is that after detached thread calls this function, mpv consider this client to be done and can do whatever. One example is when mpv exits and wait for scripts to finish we got race condition, because mpv (main thread) can exit first and the detached thread calls ta_free() which crashes, because we may have already destroyed things that the thread depends on. I've seen this crash on FreeBSD CI job (libmpv-lifetime test), where the libmpv.so has been unloaded while the thread was still doing ta_free() ``` ==5081==ERROR: AddressSanitizer: SEGV on unknoun address 0x000001111806 (pc 0x000001111806 bp 0x7fffdedf4e40 sp 0x7fffdedf4e08 T40) ==5081==The signal is caused by a READ memory access. #0 0x1111806 (<unknown module>) #1 0x803cd6900 in ta_free /home/runner/work/mpv/mpv/build/../ta/ta.c:243:5 #2 0x803a2fc87 in run_script /home/runner/work/mpv/mpv/build/../player/scripting.c:93:5 #3 0x803a2ffe0 in script_thread /home/runner/work/mpv/mpv/build/../player/scripting.c:99:5 #4 0x2c451a in asan_thread_start(void*) /usr/src/contrib/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:239:28 #5 0x80034fb04 (/lib/libthr.so.3+0x10b04) ``` Note the 0x1111806 jump which is completelly bogus. Fix this by doing mpv_destroy() as a last step in detached threads.
1 parent 22f4ff4 commit 0757185

File tree

2 files changed

+4
-2
lines changed

2 files changed

+4
-2
lines changed

input/ipc-win.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -303,8 +303,9 @@ static MP_THREAD_VOID client_thread(void *p)
303303
CloseHandle(arg->write_ol.hEvent);
304304

305305
CloseHandle(arg->client_h);
306-
mpv_destroy(arg->client);
306+
mpv_handle *client = arg->client;
307307
talloc_free(arg);
308+
mpv_destroy(client);
308309
MP_THREAD_RETURN();
309310
}
310311

player/scripting.c

+2-1
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,9 @@ static void run_script(struct mp_script_args *arg)
8989
if (arg->backend->load(arg) < 0)
9090
MP_ERR(arg, "Could not load %s script %s\n", arg->backend->name, arg->filename);
9191

92-
mpv_destroy(arg->client);
92+
mpv_handle *client = arg->client;
9393
talloc_free(arg);
94+
mpv_destroy(client);
9495
}
9596

9697
static MP_THREAD_VOID script_thread(void *p)

0 commit comments

Comments
 (0)