Skip to content

Commit

Permalink
Kill the server only after a small delay
Browse files Browse the repository at this point in the history
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 <#1992>
  • Loading branch information
rom1v committed Jan 1, 2021
1 parent 05e8c1a commit 10b749e
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 2 deletions.
46 changes: 44 additions & 2 deletions app/src/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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);
}
5 changes: 5 additions & 0 deletions app/src/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 10b749e

Please sign in to comment.