From c996a6d462d7499b1421eabcfcadf0a58baae60e Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Thu, 27 Jan 2022 23:22:43 +0100 Subject: [PATCH] Fix socket close race condition The server needs to interrupt the sockets on stop, but it must not close them while other threads may attempt to read from or write to them. In particular, the video_socket is read by the stream thread, and the control_socket is written by the controller and read by receiver. Therefore, close the socket only on sc_server_destroy(), which is called after all other threads are joined. Reported by TSAN on close: WARNING: ThreadSanitizer: data race (pid=3287612) Write of size 8 at 0x7ba000000080 by thread T1: #0 close ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1690 (libtsan.so.0+0x359d8) #1 net_close ../app/src/util/net.c:280 (scrcpy+0x23643) #2 run_server ../app/src/server.c:772 (scrcpy+0x20047) #3 (libSDL2-2.0.so.0+0x905a0) Previous read of size 8 at 0x7ba000000080 by thread T16: #0 recv ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:6603 (libtsan.so.0+0x4f4a6) #1 net_recv_all ../app/src/util/net.c:228 (scrcpy+0x234a9) #2 stream_recv_packet ../app/src/stream.c:33 (scrcpy+0x2045c) #3 run_stream ../app/src/stream.c:228 (scrcpy+0x21169) #4 (libSDL2-2.0.so.0+0x905a0) Refs ddb9396743072f97628fab168ef7fcd45a597b03 --- app/src/server.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/src/server.c b/app/src/server.c index b62a74f1..e5acef95 100644 --- a/app/src/server.c +++ b/app/src/server.c @@ -769,12 +769,10 @@ run_server(void *data) { // Interrupt sockets to wake up socket blocking calls on the server assert(server->video_socket != SC_SOCKET_NONE); net_interrupt(server->video_socket); - net_close(server->video_socket); if (server->control_socket != SC_SOCKET_NONE) { // There is no control_socket if --no-control is set net_interrupt(server->control_socket); - net_close(server->control_socket); } // Give some delay for the server to terminate properly @@ -830,6 +828,13 @@ sc_server_stop(struct sc_server *server) { void sc_server_destroy(struct sc_server *server) { + if (server->video_socket != SC_SOCKET_NONE) { + net_close(server->video_socket); + } + if (server->control_socket != SC_SOCKET_NONE) { + net_close(server->control_socket); + } + sc_server_params_destroy(&server->params); sc_intr_destroy(&server->intr); sc_cond_destroy(&server->cond_stopped);