From 10b749e27d34f91d04ff165134fda23e6ec2cc3b Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Fri, 1 Jan 2021 12:41:25 +0100 Subject: [PATCH] Kill the server only after a small delay Let the server terminate properly once all the sockets are closed. If it does not terminate (this can happen if the device is asleep), then kill it. Note: since the server process termination is detected by a flag set after waitpid() returns, there is a small chance that the process terminates (and the PID assigned to a new process) before the flag is set but before the kill() call. This race condition already existed before this commit. Fixes #1992 --- app/src/server.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- app/src/server.h | 5 +++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/app/src/server.c b/app/src/server.c index 42e633af..cac7b367 100644 --- a/app/src/server.c +++ b/app/src/server.c @@ -11,6 +11,7 @@ #include "config.h" #include "command.h" +#include "util/lock.h" #include "util/log.h" #include "util/net.h" #include "util/str_util.h" @@ -361,6 +362,19 @@ server_init(struct server *server) { atomic_flag_clear_explicit(&server->server_socket_closed, memory_order_relaxed); + server->mutex = SDL_CreateMutex(); + if (!server->mutex) { + return false; + } + + server->process_terminated_cond = SDL_CreateCond(); + if (!server->process_terminated_cond) { + SDL_DestroyMutex(server->mutex); + return false; + } + + server->process_terminated = false; + server->server_socket = INVALID_SOCKET; server->video_socket = INVALID_SOCKET; server->control_socket = INVALID_SOCKET; @@ -379,6 +393,12 @@ static int run_wait_server(void *data) { struct server *server = data; cmd_simple_wait(server->process, NULL); // ignore exit code + + mutex_lock(server->mutex); + server->process_terminated = true; + cond_signal(server->process_terminated_cond); + mutex_unlock(server->mutex); + // no need for synchronization, server_socket is initialized before this // thread was created if (server->server_socket != INVALID_SOCKET @@ -510,17 +530,39 @@ server_stop(struct server *server) { assert(server->process != PROCESS_NONE); - cmd_terminate(server->process); - if (server->tunnel_enabled) { // ignore failure disable_tunnel(server); } + // Give some delay for the server to terminate properly + mutex_lock(server->mutex); + int r = 0; + if (!server->process_terminated) { +#define WATCHDOG_DELAY_MS 1000 + r = cond_wait_timeout(server->process_terminated_cond, + server->mutex, + WATCHDOG_DELAY_MS); + } + mutex_unlock(server->mutex); + + // After this delay, kill the server if it's not dead already. + // On some devices, closing the sockets is not sufficient to wake up the + // blocking calls while the device is asleep. + if (r == SDL_MUTEX_TIMEDOUT) { + // FIXME There is a race condition here: there is a small chance that + // the process is already terminated, and the PID assigned to a new + // process. + LOGW("Killing the server..."); + cmd_terminate(server->process); + } + SDL_WaitThread(server->wait_server_thread, NULL); } void server_destroy(struct server *server) { SDL_free(server->serial); + SDL_DestroyCond(server->process_terminated_cond); + SDL_DestroyMutex(server->mutex); } diff --git a/app/src/server.h b/app/src/server.h index 823db0ea..9b558aee 100644 --- a/app/src/server.h +++ b/app/src/server.h @@ -18,6 +18,11 @@ struct server { process_t process; SDL_Thread *wait_server_thread; atomic_flag server_socket_closed; + + SDL_mutex *mutex; + SDL_cond *process_terminated_cond; + bool process_terminated; + socket_t server_socket; // only used if !tunnel_forward socket_t video_socket; socket_t control_socket;