Skip to content

Commit

Permalink
Expose socket interruption
Browse files Browse the repository at this point in the history
On Linux, socket functions are unblocked by shutdown(), but on Windows
they are unblocked by closesocket().

Expose net_interrupt() and net_close() to abstract these differences:
 - net_interrupt() calls shutdown() on Linux and closesocket() on
   Windows (if not already called);
 - net_close() calls close() on Linux and closesocket() on Windows (if
   not already called).

This simplifies the server code, and prevents a data race on close
(reported by TSAN) on Linux (but does not fix it on Windows):

    WARNING: ThreadSanitizer: data race (pid=836124)
      Write of size 8 at 0x7ba0000000d0 by main thread:
        #0 close ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1690 (libtsan.so.0+0x359d8)
        #1 net_close ../app/src/util/net.c:211 (scrcpy+0x1c76b)
        #2 close_socket ../app/src/server.c:330 (scrcpy+0x19442)
        #3 server_stop ../app/src/server.c:522 (scrcpy+0x19e33)
        #4 scrcpy ../app/src/scrcpy.c:532 (scrcpy+0x156fc)
        #5 main ../app/src/main.c:92 (scrcpy+0x622a)

      Previous read of size 8 at 0x7ba0000000d0 by thread T6:
        #0 recv ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:6603 (libtsan.so.0+0x4f4a6)
        #1 net_recv ../app/src/util/net.c:167 (scrcpy+0x1c5a7)
        #2 run_receiver ../app/src/receiver.c:76 (scrcpy+0x12819)
        #3 <null> <null> (libSDL2-2.0.so.0+0x84f40)
  • Loading branch information
rom1v committed Oct 26, 2021
1 parent e5ea137 commit 106a3df
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 42 deletions.
68 changes: 35 additions & 33 deletions app/src/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -323,21 +323,10 @@ connect_to_server(uint16_t port, uint32_t attempts, uint32_t delay) {
return SC_INVALID_SOCKET;
}

static void
close_socket(sc_socket socket) {
assert(socket != SC_INVALID_SOCKET);
net_shutdown(socket, SHUT_RDWR);
if (!net_close(socket)) {
LOGW("Could not close socket");
}
}

bool
server_init(struct server *server) {
server->serial = NULL;
server->process = PROCESS_NONE;
atomic_flag_clear_explicit(&server->server_socket_closed,
memory_order_relaxed);

bool ok = sc_mutex_init(&server->mutex);
if (!ok) {
Expand Down Expand Up @@ -376,12 +365,11 @@ run_wait_server(void *data) {

// no need for synchronization, server_socket is initialized before this
// thread was created
if (server->server_socket != SC_INVALID_SOCKET
&& !atomic_flag_test_and_set(&server->server_socket_closed)) {
// On Linux, accept() is unblocked by shutdown(), but on Windows, it is
// unblocked by closesocket(). Therefore, call both (close_socket()).
close_socket(server->server_socket);
if (server->server_socket != SC_INVALID_SOCKET) {
// Unblock any accept()
net_interrupt(server->server_socket);
}

LOGD("Server terminated");
return 0;
}
Expand Down Expand Up @@ -430,14 +418,8 @@ server_start(struct server *server, const struct server_params *params) {
return true;

error:
if (!server->tunnel_forward) {
bool was_closed =
atomic_flag_test_and_set(&server->server_socket_closed);
// the thread is not started, the flag could not be already set
assert(!was_closed);
(void) was_closed;
close_socket(server->server_socket);
}
// The server socket (if any) will be closed on server_destroy()

disable_tunnel(server);

return false;
Expand Down Expand Up @@ -479,11 +461,11 @@ server_connect_to(struct server *server, char *device_name, struct size *size) {
}

// we don't need the server socket anymore
if (!atomic_flag_test_and_set(&server->server_socket_closed)) {
// close it from here
close_socket(server->server_socket);
// otherwise, it is closed by run_wait_server()
if (!net_close(server->server_socket)) {
LOGW("Could not close server socket on connect");
}
// Do not attempt to close it again on server_destroy()
server->server_socket = SC_INVALID_SOCKET;
} else {
uint32_t attempts = 100;
uint32_t delay = 100; // ms
Expand Down Expand Up @@ -511,15 +493,20 @@ server_connect_to(struct server *server, char *device_name, struct size *size) {

void
server_stop(struct server *server) {
if (server->server_socket != SC_INVALID_SOCKET
&& !atomic_flag_test_and_set(&server->server_socket_closed)) {
close_socket(server->server_socket);
if (server->server_socket != SC_INVALID_SOCKET) {
if (!net_interrupt(server->server_socket)) {
LOGW("Could not interrupt server socket");
}
}
if (server->video_socket != SC_INVALID_SOCKET) {
close_socket(server->video_socket);
if (!net_interrupt(server->video_socket)) {
LOGW("Could not interrupt video socket");
}
}
if (server->control_socket != SC_INVALID_SOCKET) {
close_socket(server->control_socket);
if (!net_interrupt(server->control_socket)) {
LOGW("Could not interrupt control socket");
}
}

assert(server->process != PROCESS_NONE);
Expand Down Expand Up @@ -556,6 +543,21 @@ server_stop(struct server *server) {

void
server_destroy(struct server *server) {
if (server->server_socket != SC_INVALID_SOCKET) {
if (!net_close(server->server_socket)) {
LOGW("Could not close server socket");
}
}
if (server->video_socket != SC_INVALID_SOCKET) {
if (!net_close(server->video_socket)) {
LOGW("Could not close video socket");
}
}
if (server->control_socket != SC_INVALID_SOCKET) {
if (!net_close(server->control_socket)) {
LOGW("Could not close control socket");
}
}
free(server->serial);
sc_cond_destroy(&server->process_terminated_cond);
sc_mutex_destroy(&server->mutex);
Expand Down
1 change: 0 additions & 1 deletion app/src/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ struct server {
char *serial;
process_t process;
sc_thread wait_server_thread;
atomic_flag server_socket_closed;

sc_mutex mutex;
sc_cond process_terminated_cond;
Expand Down
23 changes: 20 additions & 3 deletions app/src/util/net.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "net.h"

#include <assert.h>
#include <stdio.h>
#include <SDL2/SDL_platform.h>

Expand Down Expand Up @@ -55,6 +56,7 @@ wrap(sc_raw_socket sock) {
}

socket->socket = sock;
socket->closed = (atomic_flag) ATOMIC_FLAG_INIT;

return socket;
#else
Expand Down Expand Up @@ -195,18 +197,33 @@ net_send_all(sc_socket socket, const void *buf, size_t len) {
}

bool
net_shutdown(sc_socket socket, int how) {
net_interrupt(sc_socket socket) {
assert(socket != SC_INVALID_SOCKET);

sc_raw_socket raw_sock = unwrap(socket);
return !shutdown(raw_sock, how);

#ifdef __WINDOWS__
if (!atomic_flag_test_and_set(&socket->closed)) {
return !closesocket(raw_sock);
}
return true;
#else
return !shutdown(raw_sock, SHUT_RDWR);
#endif
}

#include <errno.h>
bool
net_close(sc_socket socket) {
sc_raw_socket raw_sock = unwrap(socket);

#ifdef __WINDOWS__
bool ret = true;
if (!atomic_flag_test_and_set(&socket->closed)) {
ret = !closesocket(raw_sock);
}
free(socket);
return !closesocket(raw_sock);
return ret;
#else
return !close(raw_sock);
#endif
Expand Down
12 changes: 7 additions & 5 deletions app/src/util/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@
#ifdef __WINDOWS__

# include <winsock2.h>
# define SHUT_RD SD_RECEIVE
# define SHUT_WR SD_SEND
# define SHUT_RDWR SD_BOTH
# include <stdatomic.h>
# define SC_INVALID_SOCKET NULL
typedef struct sc_socket_windows {
SOCKET socket;
atomic_flag closed;
} *sc_socket;

#else // not __WINDOWS__
Expand Down Expand Up @@ -53,10 +52,13 @@ net_send(sc_socket socket, const void *buf, size_t len);
ssize_t
net_send_all(sc_socket socket, const void *buf, size_t len);

// how is SHUT_RD (read), SHUT_WR (write) or SHUT_RDWR (both)
// Shutdown the socket (or close on Windows) so that any blocking send() or
// recv() are interrupted
bool
net_shutdown(sc_socket socket, int how);
net_interrupt(sc_socket socket);

// Close the socket.
// A socket must always be closed, even if net_interrupt() has been called.
bool
net_close(sc_socket socket);

Expand Down

0 comments on commit 106a3df

Please sign in to comment.