Compare commits

..

4 Commits

Author SHA1 Message Date
Romain Vimont
73d098872c Do not warn on terminating the server
If the server is already dead, terminating it fails. This is expected.
2020-03-29 11:24:11 +02:00
Romain Vimont
943d264b35 Do not block on accept() if server died
The server may die before connecting to the client. In that case, the
client was blocked indefinitely (until Ctrl+C) on accept().

To avoid the problem, use a pipe and a select() before calling accept(),
so that the blocking call can be interrupted. Once accept() is called,
it is guaranteed not to block.
2020-03-29 11:24:11 +02:00
Romain Vimont
32dc192883 Wait server from a separate thread
Create a thread just to wait for the server process exit.

This paves the way to simply wake up a blocking accept() in a portable
way.
2020-03-28 23:56:37 +01:00
Romain Vimont
7732f1097b Refactor server_start() error handling
This will avoid more cleanup duplication.
2020-03-28 23:56:37 +01:00
9 changed files with 112 additions and 76 deletions

View File

@@ -23,7 +23,7 @@ fps_counter_init(struct fps_counter *counter) {
}
counter->thread = NULL;
atomic_init(&counter->started, 0);
SDL_AtomicSet(&counter->started, 0);
// no need to initialize the other fields, they are unused until started
return true;
@@ -35,16 +35,6 @@ fps_counter_destroy(struct fps_counter *counter) {
SDL_DestroyMutex(counter->mutex);
}
static inline bool
is_started(struct fps_counter *counter) {
return atomic_load_explicit(&counter->started, memory_order_acquire);
}
static inline void
set_started(struct fps_counter *counter, bool started) {
atomic_store_explicit(&counter->started, started, memory_order_release);
}
// must be called with mutex locked
static void
display_fps(struct fps_counter *counter) {
@@ -80,10 +70,10 @@ run_fps_counter(void *data) {
mutex_lock(counter->mutex);
while (!counter->interrupted) {
while (!counter->interrupted && !is_started(counter)) {
while (!counter->interrupted && !SDL_AtomicGet(&counter->started)) {
cond_wait(counter->state_cond, counter->mutex);
}
while (!counter->interrupted && is_started(counter)) {
while (!counter->interrupted && SDL_AtomicGet(&counter->started)) {
uint32_t now = SDL_GetTicks();
check_interval_expired(counter, now);
@@ -106,7 +96,7 @@ fps_counter_start(struct fps_counter *counter) {
counter->nr_skipped = 0;
mutex_unlock(counter->mutex);
set_started(counter, true);
SDL_AtomicSet(&counter->started, 1);
cond_signal(counter->state_cond);
// counter->thread is always accessed from the same thread, no need to lock
@@ -124,13 +114,13 @@ fps_counter_start(struct fps_counter *counter) {
void
fps_counter_stop(struct fps_counter *counter) {
set_started(counter, false);
SDL_AtomicSet(&counter->started, 0);
cond_signal(counter->state_cond);
}
bool
fps_counter_is_started(struct fps_counter *counter) {
return is_started(counter);
return SDL_AtomicGet(&counter->started);
}
void
@@ -155,7 +145,7 @@ fps_counter_join(struct fps_counter *counter) {
void
fps_counter_add_rendered_frame(struct fps_counter *counter) {
if (!is_started(counter)) {
if (!SDL_AtomicGet(&counter->started)) {
return;
}
@@ -168,7 +158,7 @@ fps_counter_add_rendered_frame(struct fps_counter *counter) {
void
fps_counter_add_skipped_frame(struct fps_counter *counter) {
if (!is_started(counter)) {
if (!SDL_AtomicGet(&counter->started)) {
return;
}

View File

@@ -1,9 +1,9 @@
#ifndef FPSCOUNTER_H
#define FPSCOUNTER_H
#include <stdatomic.h>
#include <stdbool.h>
#include <stdint.h>
#include <SDL2/SDL_atomic.h>
#include <SDL2/SDL_mutex.h>
#include <SDL2/SDL_thread.h>
@@ -16,7 +16,7 @@ struct fps_counter {
// atomic so that we can check without locking the mutex
// if the FPS counter is disabled, we don't want to lock unnecessarily
atomic_bool started;
SDL_atomic_t started;
// the following fields are protected by the mutex
bool interrupted;

View File

@@ -5,6 +5,7 @@
#include <inttypes.h>
#include <libgen.h>
#include <stdio.h>
#include <unistd.h>
#include <SDL2/SDL_thread.h>
#include <SDL2/SDL_timer.h>
#include <SDL2/SDL_platform.h>
@@ -318,12 +319,14 @@ connect_to_server(uint16_t port, uint32_t attempts, uint32_t delay) {
}
static void
close_socket(socket_t socket) {
assert(socket != INVALID_SOCKET);
net_shutdown(socket, SHUT_RDWR);
if (!net_close(socket)) {
close_socket(socket_t *socket) {
assert(*socket != INVALID_SOCKET);
net_shutdown(*socket, SHUT_RDWR);
if (!net_close(*socket)) {
LOGW("Could not close socket");
return;
}
*socket = INVALID_SOCKET;
}
void
@@ -335,14 +338,10 @@ static int
run_wait_server(void *data) {
struct server *server = data;
cmd_simple_wait(server->process, NULL); // ignore exit code
// no need for synchronization, server_socket is initialized before this
// thread was created
if (server->server_socket != 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);
}
// wake up any net_select_interruptible()
close(server->pipe_intr[1]);
LOGD("Server terminated");
return 0;
}
@@ -367,37 +366,36 @@ server_start(struct server *server, const char *serial,
goto error1;
}
// server will connect to our server socket
server->process = execute_server(server, params);
if (server->process == PROCESS_NONE) {
bool ok = net_pipe(server->pipe_intr);
if (!ok) {
perror("pipe");
goto error2;
}
// If the server process dies before connecting to the server socket, then
// the client will be stuck forever on accept(). To avoid the problem, we
// must be able to wake up the accept() call when the server dies. To keep
// things simple and multiplatform, just spawn a new thread waiting for the
// server process and calling shutdown()/close() on the server socket if
// necessary to wake up any accept() blocking call.
// server will connect to our server socket
server->process = execute_server(server, params);
if (server->process == PROCESS_NONE) {
goto error3;
}
server->wait_server_thread =
SDL_CreateThread(run_wait_server, "wait-server", server);
if (!server->wait_server_thread) {
cmd_terminate(server->process);
cmd_simple_wait(server->process, NULL); // ignore exit code
goto error2;
goto error3;
}
server->tunnel_enabled = true;
return true;
error3:
close(server->pipe_intr[0]);
close(server->pipe_intr[1]);
error2:
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);
close_socket(server->server_socket);
close_socket(&server->server_socket);
}
disable_tunnel(server);
error1:
@@ -408,23 +406,34 @@ error1:
bool
server_connect_to(struct server *server) {
if (!server->tunnel_forward) {
bool acceptable = net_select_interruptible(server->server_socket,
server->pipe_intr[0]);
if (!acceptable) {
// the process died, accept() would never succeed
return false;
}
server->video_socket = net_accept(server->server_socket);
if (server->video_socket == INVALID_SOCKET) {
return false;
}
acceptable = net_select_interruptible(server->server_socket,
server->pipe_intr[0]);
if (!acceptable) {
// the process died, accept() would never succeed
return false;
}
server->control_socket = net_accept(server->server_socket);
if (server->control_socket == INVALID_SOCKET) {
// the video_socket will be cleaned up on destroy
return false;
}
close(server->pipe_intr[0]);
// 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()
}
close_socket(&server->server_socket);
} else {
uint32_t attempts = 100;
uint32_t delay = 100; // ms
@@ -451,15 +460,14 @@ server_connect_to(struct server *server) {
void
server_stop(struct server *server) {
if (server->server_socket != INVALID_SOCKET
&& !atomic_flag_test_and_set(&server->server_socket_closed)) {
close_socket(server->server_socket);
if (server->server_socket != INVALID_SOCKET) {
close_socket(&server->server_socket);
}
if (server->video_socket != INVALID_SOCKET) {
close_socket(server->video_socket);
close_socket(&server->video_socket);
}
if (server->control_socket != INVALID_SOCKET) {
close_socket(server->control_socket);
close_socket(&server->control_socket);
}
assert(server->process != PROCESS_NONE);

View File

@@ -1,7 +1,6 @@
#ifndef SERVER_H
#define SERVER_H
#include <stdatomic.h>
#include <stdbool.h>
#include <stdint.h>
#include <SDL2/SDL_thread.h>
@@ -15,7 +14,7 @@ struct server {
char *serial;
process_t process;
SDL_Thread *wait_server_thread;
atomic_flag server_socket_closed;
int pipe_intr[2]; // to wake up blocking accept() on process exit
socket_t server_socket; // only used if !tunnel_forward
socket_t video_socket;
socket_t control_socket;
@@ -28,8 +27,6 @@ struct server {
#define SERVER_INITIALIZER { \
.serial = NULL, \
.process = PROCESS_NONE, \
.wait_server_thread = NULL, \
.server_socket_closed = ATOMIC_FLAG_INIT, \
.server_socket = INVALID_SOCKET, \
.video_socket = INVALID_SOCKET, \
.control_socket = INVALID_SOCKET, \

View File

@@ -1,16 +1,22 @@
#include "net.h"
#include <assert.h>
#include <errno.h>
#include <stdio.h>
#include <SDL2/SDL_platform.h>
#include "config.h"
#include "common.h"
#include "log.h"
#ifdef __WINDOWS__
# include <io.h>
# include <winsock2.h>
typedef int socklen_t;
#else
# include <sys/types.h>
# include <sys/select.h>
# include <sys/socket.h>
# include <sys/types.h>
# include <netinet/in.h>
# include <arpa/inet.h>
# include <unistd.h>
@@ -145,3 +151,38 @@ net_close(socket_t socket) {
return !close(socket);
#endif
}
bool
net_select_interruptible(int fd, int fd_intr) {
fd_set rfds;
FD_ZERO(&rfds);
FD_SET(fd, &rfds);
FD_SET(fd_intr, &rfds);
int nfds = MAX(fd, fd_intr) + 1;
// use select() because it's available on supported platforms
int r = select(nfds, &rfds, NULL, NULL, NULL);
if (r == -1) {
// failure
return false;
}
assert(r > 0);
if (FD_ISSET(fd_intr, &rfds)) {
// interrupted is set
return false;
}
assert(FD_ISSET(fd, &rfds));
return true;
}
bool
net_pipe(int fds[static 2]) {
#ifdef __WINDOWS__
return !_pipe(fds, 4096, 0);
#else
return !pipe(fds);
#endif
}

View File

@@ -54,4 +54,12 @@ net_shutdown(socket_t socket, int how);
bool
net_close(socket_t socket);
// wait for fd or fd_intr to be readable
// return true if fd is readable and fd_intr is not
bool
net_select_interruptible(int fd, int fd_intr);
bool
net_pipe(int fd[static 2]);
#endif

View File

@@ -163,10 +163,8 @@ public final class Device {
}
public void setClipboardText(String text) {
boolean ok = serviceManager.getClipboardManager().setText(text);
if (ok) {
Ln.i("Device clipboard set");
}
serviceManager.getClipboardManager().setText(text);
Ln.i("Device clipboard set");
}
/**
@@ -178,10 +176,8 @@ public final class Device {
Ln.e("Could not get built-in display");
return;
}
boolean ok = SurfaceControl.setDisplayPowerMode(d, mode);
if (ok) {
Ln.i("Device screen turned " + (mode == Device.POWER_MODE_OFF ? "off" : "on"));
}
SurfaceControl.setDisplayPowerMode(d, mode);
Ln.i("Device screen turned " + (mode == Device.POWER_MODE_OFF ? "off" : "on"));
}
/**

View File

@@ -74,15 +74,13 @@ public class ClipboardManager {
}
}
public boolean setText(CharSequence text) {
public void setText(CharSequence text) {
try {
Method method = getSetPrimaryClipMethod();
ClipData clipData = ClipData.newPlainText(null, text);
setPrimaryClip(method, manager, clipData);
return true;
} catch (InvocationTargetException | IllegalAccessException | NoSuchMethodException e) {
Ln.e("Could not invoke method", e);
return false;
}
}
}

View File

@@ -121,14 +121,12 @@ public final class SurfaceControl {
return setDisplayPowerModeMethod;
}
public static boolean setDisplayPowerMode(IBinder displayToken, int mode) {
public static void setDisplayPowerMode(IBinder displayToken, int mode) {
try {
Method method = getSetDisplayPowerModeMethod();
method.invoke(null, displayToken, mode);
return true;
} catch (InvocationTargetException | IllegalAccessException | NoSuchMethodException e) {
Ln.e("Could not invoke method", e);
return false;
}
}