From patchwork Wed Nov 11 04:28:06 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Hatle X-Patchwork-Id: 103646 Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) by mx.groups.io with SMTP id smtpd.msgproc02-g2.2563.1794371289099913999 for ; Tue, 10 Nov 2026 20:28:09 -0800 Authentication-Results: mx.groups.io; dkim=none (message not signed); spf=pass (domain: kernel.crashing.org, ip: 63.228.1.57, mailfrom: mark.hatle@kernel.crashing.org) Received: from kernel.crashing.net (70-99-78-136.nuveramail.net [70.99.78.136] (may be forged)) by gate.crashing.org (8.18.1/8.18.1/Debian-2) with ESMTP id 6AB4S8mF2429215 for ; Tue, 10 Nov 2026 22:28:08 -0600 From: Mark Hatle To: yocto-patches@lists.yoctoproject.org Subject: [pseudo][PATCH 1/2] Send client requests over one shared datagram socket Date: Tue, 10 Nov 2026 22:28:06 -0600 Message-Id: <1794371287-31540-2-git-send-email-mark.hatle@kernel.crashing.org> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1794371287-31540-1-git-send-email-mark.hatle@kernel.crashing.org> References: <1794371287-31540-1-git-send-email-mark.hatle@kernel.crashing.org> List-Id: X-Webhook-Received: from 45-33-107-173.ip.linodeusercontent.com [45.33.107.173] by lin-tor-korg-lkml-alma10-1 with HTTPS for ; Wed, 11 Nov 2026 04:28:17 -0000 X-Groupsio-URL: https://lists.yoctoproject.org/g/yocto-patches/message/5295 MIME-Version: 1.0 From: Mark Hatle The server handles each client's stream connection separately, so it has no way to tell the order of requests queued by different clients. That doesn't matter while every client waits for each reply, but it prevents sending requests without waiting: a process could change a file, tell another process, and that process's question could be answered first. Have clients send all requests, after the initial PING, to a datagram socket shared by all clients, pseudo.request. The kernel queues datagrams in the order they were sent, so the server handles requests in that order even across processes. Replies still go back over each client's stream connection, which also carries PING and SHUTDOWN and tells the server when a client goes away. Requests carry the client's slot and a per-connection serial number, returned by the PING, so a request still queued from a client that has gone away is applied but never answered on a reused slot. The server reads each request with one recv() instead of FIONREAD plus two reads, and drains the socket before exiting. AI-Generated: Uses GitHub Copilot Signed-off-by: Mark Hatle --- README.security | 4 +- doc/overview | 3 +- doc/pseudo_ipc | 36 +++++++++++++ pseudo.h | 1 + pseudo_client.c | 150 +++++++++++++++++++++++++++++++++++++----------------- pseudo_ipc.c | 42 ++++++++++++++++ pseudo_ipc.h | 8 +++ pseudo_server.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 8 files changed, 348 insertions(+), 50 deletions(-) diff --git a/README.security b/README.security index 45533f2..5564448 100644 --- a/README.security +++ b/README.security @@ -133,9 +133,9 @@ ignore mismatch errors in pseudo's log. 6. THE SERVER TRUSTS ITS CLIENTS -The pseudo server listens on a Unix socket in its local state directory +The pseudo server listens on Unix sockets in its local state directory and does not authenticate clients beyond the filesystem permissions of -that socket. Any process running as the build user can connect and +those sockets. Any process running as the build user can connect and change the database, and the database files themselves are writable by that user. The resulting metadata can only be trusted as much as every process running as the build user. diff --git a/doc/overview b/doc/overview index 96c883d..c9497e5 100644 --- a/doc/overview +++ b/doc/overview @@ -19,7 +19,8 @@ to the virtual environment. Underlying this is access to a server process, which is automatically spawned by the library if one is not available. The server process maintains -a UNIX domain socket while it is active, and maintains a database (using +UNIX domain sockets while it is active (see doc/pseudo_ipc), and maintains a +database (using sqlite) of files known to the system. Files are recorded in the database only if they are created within the virtualized environment or have been altered by it; files merely read are not added. diff --git a/doc/pseudo_ipc b/doc/pseudo_ipc index 0d9d6cd..e4ec750 100644 --- a/doc/pseudo_ipc +++ b/doc/pseudo_ipc @@ -1,5 +1,28 @@ MESSAGE PASSING +The server has two UNIX domain sockets in its local state directory: + +pseudo.socket + A stream socket. Each client connects to it, sends a PING to + register, and gets every reply on this connection. pseudo -S + also sends its SHUTDOWN request here, and the server notices a + client going away when the connection closes. + +pseudo.request + A datagram socket shared by all clients. Every request after the + initial PING is sent here. + +The kernel queues datagrams from all senders on the one socket in the +order they were sent, so the server handles requests in that order even +across processes. Once a client's send has returned, its request is +queued ahead of anything another client sends afterwards. If a process +changes a file and then tells another process about it (through a pipe, +by exiting, or any other way), the server has already queued the change +by the time the second process can ask about the file. + +It also means the server reads each request with a single recv(), +rather than a header and then a path. + typedef struct { pseudo_msg_type_t type; pseudo_op_t op; @@ -16,6 +39,8 @@ typedef struct { unsigned int pathlen; int nlink; int deleting; + int client_id; + unsigned int client_serial; char path[]; } pseudo_msg_t; @@ -44,6 +69,11 @@ only sends PING, OP, or FASTOP. FASTOP is used for operations whose result the client doesn't need, but the server still replies with ACK (or NAK on failure) and the client still waits for that reply. +A PING on the stream socket registers the client: the reply gives it the +client_id and client_serial to put in its requests. A PING on the +request socket is answered once every request the client sent before it +has been handled. + When run with '-S', the pseudo program runs as a client, sending a SHUTDOWN message to a server -- but only if it can find one, it does not start a new one. In this case, the server @@ -84,6 +114,12 @@ deleting carries the database's "deleting" mark (see doc/database) when the server looks up file entries. The server uses it internally; the client doesn't set or use it. +client_id and client_serial identify the client's stream connection in +requests on the shared socket, so the server knows where to reply. The +server numbers every connection, so a request still queued from a client +that has gone away is never answered on a connection that has since +reused its client_id; the server applies it without replying. + 32/64 bit: This structure should have the same offsets for every element, including path, on both 64-bit and 32-bit machines. (Check with 'offsets.c'.) It is *not* an error if sizeof(pseudo_msg_t) is different; the padding diff --git a/pseudo.h b/pseudo.h index 6a7339b..253a21d 100644 --- a/pseudo.h +++ b/pseudo.h @@ -131,6 +131,7 @@ extern char *pseudo_version; #define PSEUDO_PATHLOCK "pseudo.pathlock" #define PSEUDO_PIDFILE "pseudo.pid" #define PSEUDO_SOCKET "pseudo.socket" +#define PSEUDO_REQUEST_SOCKET "pseudo.request" #define PSEUDO_SEVERITY_DEFAULT "info,warn,error" diff --git a/pseudo_client.c b/pseudo_client.c index 58e8810..c5b960f 100644 --- a/pseudo_client.c +++ b/pseudo_client.c @@ -46,6 +46,13 @@ static char *program_invocation_name = "unknown"; static char *base_path(int dirfd, const char *path, int leave_last); static int connect_fd = -1; +/* Requests go to the server over a datagram socket shared by all + * clients, so the server handles them in the order they were sent even + * across processes; replies come back on connect_fd. See pseudo_server.c. + */ +static int request_fd = -1; +static int client_id = 0; +static unsigned int client_serial = 0; static int server_pid = 0; int pseudo_localstate_dir_fd = -1; static int pseudo_pathlock_fd = -1; @@ -429,6 +436,18 @@ pseudo_profile_report(void) { } #endif +static void +client_disconnect(void) { + if (connect_fd != -1) { + close(connect_fd); + connect_fd = -1; + } + if (request_fd != -1) { + close(request_fd); + request_fd = -1; + } +} + void pseudo_init_client(void) { char *env; @@ -436,10 +455,7 @@ pseudo_init_client(void) { pseudo_antimagic(); pseudo_new_pid(); - if (connect_fd != -1) { - close(connect_fd); - connect_fd = -1; - } + client_disconnect(); #ifdef PSEUDO_PROFILING if (pseudo_profile_fd > -1) { close(pseudo_profile_fd); @@ -1110,6 +1126,8 @@ client_ping(void) { server_pid = 0; return 1; } else { + client_id = ack->client_id; + client_serial = ack->client_serial; /* The server tells us whether or not to log things. */ if (ack->result == RESULT_SUCCEED) { pseudo_client_logging = 1; @@ -1254,6 +1272,7 @@ static int client_connect(void) { /* we have a server pid, is it responsive? */ struct sockaddr_un sun = { .sun_family = AF_UNIX, .sun_path = PSEUDO_SOCKET }; + struct sockaddr_un req_sun = { .sun_family = AF_UNIX, .sun_path = PSEUDO_REQUEST_SOCKET }; int cwd_fd; connect_fd = socket(PF_UNIX, SOCK_STREAM, 0); @@ -1296,6 +1315,21 @@ client_connect(void) { connect_fd = -1; return 1; } + request_fd = pseudo_fd(socket(PF_UNIX, SOCK_DGRAM, 0), MOVE_FD); + if (request_fd == -1 || + connect(request_fd, (struct sockaddr *) &req_sun, sizeof(req_sun)) == -1) { + char *e = strerror(errno); + pseudo_debug(PDBGF_CLIENT, "Can't connect request socket to %s: (%s)\n", + PSEUDO_REQUEST_SOCKET, e); + pseudo_evlog(PDBGF_CLIENT, "request socket connect failed: %s\n", e); + client_disconnect(); + if (fchdir(cwd_fd) == -1) { + pseudo_error("return to previous directory failed: %s\n", + strerror(errno)); + } + close(cwd_fd); + return 1; + } if (fchdir(cwd_fd) == -1) { pseudo_error("return to previous directory failed: %s\n", strerror(errno)); @@ -1313,10 +1347,7 @@ pseudo_client_setup(void) { server_pid = 0; /* avoid descriptor leak, I hope */ - if (connect_fd >= 0) { - close(connect_fd); - connect_fd = -1; - } + client_disconnect(); if (client_connect() == 0) { pseudo_debug(PDBGF_CLIENT | PDBGF_VERBOSE, "connection started.\n"); if (client_ping() == 0) { @@ -1327,8 +1358,7 @@ pseudo_client_setup(void) { /* and now we'll close the connection, and try to * kick the server. */ - close(connect_fd); - connect_fd = -1; + client_disconnect(); } } pseudo_debug(PDBGF_CLIENT, "connection failed, checking server...\n"); @@ -1381,8 +1411,7 @@ pseudo_client_setup(void) { } else { pseudo_debug(PDBGF_CLIENT, "ping failed to restarted server.\n"); pseudo_evlog(PDBGF_CLIENT, "ping failed to restarted server.\n"); - close(connect_fd); - connect_fd = -1; + client_disconnect(); return 1; } } else { @@ -1394,11 +1423,57 @@ pseudo_client_setup(void) { } #define PSEUDO_RETRIES 250 + +static void +pseudo_client_abort(void) { + char *local_state_dir = pseudo_get_value("PSEUDO_LOCALSTATEDIR"); + pseudo_info("abort()ing pseudo client by server request. See https://wiki.yoctoproject.org/wiki/Pseudo_Abort for more details on this.\n" + "Check logfile: %s/%s\n", local_state_dir, PSEUDO_LOGFILE); + abort(); +} + +/* Send a request over the request socket, reconnecting to (or starting) + * the server if that fails. Returns 0 once sent. + */ +static int +pseudo_client_send(pseudo_msg_t *msg, size_t len, const char *path, int tries) { + int rc; + + for (; tries < PSEUDO_RETRIES; ++tries) { + pseudo_evlog(PDBGF_CLIENT, "try %d, request fd is %d\n", tries, request_fd); + msg->client_id = client_id; + msg->client_serial = client_serial; + rc = pseudo_msg_send(request_fd, msg, len, path); + if (rc == 0) + return tries; + pseudo_evlog(PDBGF_CLIENT, "msg_send failed [%d].\n", rc); + pseudo_debug(PDBGF_CLIENT | PDBGF_VERBOSE, "msg_send: %d%s\n", + rc, + rc == -1 ? " (sigpipe)" : + " (short write)"); + pseudo_debug(PDBGF_CLIENT, "trying to get server, try %d\n", tries); + /* try to open server; if we fail, wait a bit before + * retry. + */ + if (pseudo_client_setup()) { + int ms = (getpid() % 5) + (3 * tries); + struct timespec delay = { .tv_sec = 0, .tv_nsec = ms * 1000000 }; + pseudo_evlog(PDBGF_CLIENT, "setup failed, delaying %d ms.\n", ms); + nanosleep(&delay, NULL); + } else { + pseudo_evlog(PDBGF_CLIENT, "setup apparently successful, retrying message immediately.\n"); + } + } + pseudo_error("pseudo: server connection persistently failed, aborting.\n"); + pseudo_evlog_dump(); + pseudo_critical("event log dumped, aborting.\n"); + return -1; +} + static pseudo_msg_t * pseudo_client_request(pseudo_msg_t *msg, size_t len, const char *path) { pseudo_msg_t *response = 0; int tries = 0; - int rc; if (!msg) return 0; @@ -1410,36 +1485,16 @@ pseudo_client_request(pseudo_msg_t *msg, size_t len, const char *path) { */ pseudo_debug(PDBGF_CLIENT | PDBGF_VERBOSE, "sending a message: ino %llu\n", (unsigned long long) msg->ino); - for (tries = 0; tries < PSEUDO_RETRIES; ++tries) { - pseudo_evlog(PDBGF_CLIENT, "try %d, connect fd is %d\n", tries, connect_fd); - rc = pseudo_msg_send(connect_fd, msg, len, path); - if (rc != 0) { - pseudo_evlog(PDBGF_CLIENT, "msg_send failed [%d].\n", rc); - pseudo_debug(PDBGF_CLIENT | PDBGF_VERBOSE, "msg_send: %d%s\n", - rc, - rc == -1 ? " (sigpipe)" : - " (short write)"); - pseudo_debug(PDBGF_CLIENT, "trying to get server, try %d\n", tries); - /* try to open server; if we fail, wait a bit before - * retry. - */ - if (pseudo_client_setup()) { - int ms = (getpid() % 5) + (3 * tries); - struct timespec delay = { .tv_sec = 0, .tv_nsec = ms * 1000000 }; - pseudo_evlog(PDBGF_CLIENT, "setup failed, delaying %d ms.\n", ms); - nanosleep(&delay, NULL); - } else { - pseudo_evlog(PDBGF_CLIENT, "setup apparently successful, retrying message immediately.\n"); - } - continue; - } - /* note "continue" above; we only get here if rc was 0, - * indicating a successful send. - */ + for (; tries < PSEUDO_RETRIES; ++tries) { + tries = pseudo_client_send(msg, len, path, tries); pseudo_debug(PDBGF_CLIENT | PDBGF_VERBOSE, "sent!\n"); response = pseudo_msg_receive(connect_fd); if (!response) { pseudo_debug(PDBGF_CLIENT, "expected response did not occur; retrying\n"); + /* the connection may be gone even though the request + * socket still works, so reconnect before resending + */ + pseudo_client_setup(); } else { if (response->type != PSEUDO_MSG_ACK) { pseudo_debug(PDBGF_CLIENT, "got non-ack response %d\n", response->type); @@ -2124,6 +2179,8 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path startfd = pseudo_grp_fd + 1; if (connect_fd >= startfd) startfd = connect_fd + 1; + if (request_fd >= startfd) + startfd = request_fd + 1; /* the fds below startfd are the ones our own are mixed in * with, so close those by hand and skip the ones we need */ @@ -2132,7 +2189,7 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path i == pseudo_localstate_dir_fd || i == pseudo_pathlock_fd || i == pseudo_pwd_fd || i == pseudo_pwd_lck_fd || i == pseudo_grp_fd || - i == connect_fd) + i == connect_fd || i == request_fd) continue; pseudo_client_close(i); close(i); @@ -2152,6 +2209,11 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path if (connect_fd == -1) { pseudo_error("tried to close connection, couldn't dup: %s\n", strerror(errno)); } + } else if (fd == request_fd) { + request_fd = pseudo_fd(request_fd, COPY_FD); + if (request_fd == -1) { + pseudo_error("tried to close request socket, couldn't dup: %s\n", strerror(errno)); + } } else if (fd == pseudo_util_debug_fd || fd == pseudo_util_evlog_fd) { /* both are stderr by default */ int newfd = pseudo_fd(fd, COPY_FD); @@ -2257,12 +2319,8 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path #endif if (result) { pseudo_debug(PDBGF_OP, "(%d) %s", getpid(), pseudo_res_name(result->result)); - if (result->result == RESULT_ABORT) { - char *local_state_dir = pseudo_get_value("PSEUDO_LOCALSTATEDIR"); - pseudo_info("abort()ing pseudo client by server request. See https://wiki.yoctoproject.org/wiki/Pseudo_Abort for more details on this.\n" - "Check logfile: %s/%s\n", local_state_dir, PSEUDO_LOGFILE); - abort(); - } + if (result->result == RESULT_ABORT) + pseudo_client_abort(); if (op == OP_STAT || op == OP_FSTAT) { pseudo_debug(PDBGF_OP, " mode 0%o uid %d:%d", (int) result->mode, diff --git a/pseudo_ipc.c b/pseudo_ipc.c index e51fe0e..e3dd064 100644 --- a/pseudo_ipc.c +++ b/pseudo_ipc.c @@ -188,6 +188,48 @@ pseudo_msg_receive(int fd) { return incoming; } +/* receive one request from the server's datagram socket without + * blocking. Returns NULL if none is queued (errno EAGAIN), or if the + * datagram was malformed and has been discarded (errno EBADMSG). + */ +pseudo_msg_t * +pseudo_msg_receive_request(int fd) { + static pseudo_msg_t *incoming; + ssize_t r; + + if (fd < 0) + return 0; + if (!incoming) { + /* one spare byte so the path can always be null-terminated */ + incoming = malloc(PSEUDO_MAX_REQUEST + 1); + if (!incoming) { + pseudo_error("Couldn't allocate request buffer.\n"); + return 0; + } + } + errno = 0; + r = recv(fd, incoming, PSEUDO_MAX_REQUEST, MSG_DONTWAIT | MSG_TRUNC); + if (r == -1) { + if (errno != EAGAIN && errno != EWOULDBLOCK) + pseudo_debug(PDBGF_IPC, "request recv failed: %s\n", strerror(errno)); + return 0; + } + if (r > PSEUDO_MAX_REQUEST) { + pseudo_error("dropped oversized request (%ld bytes).\n", (long) r); + errno = EBADMSG; + return 0; + } + if (r < (ssize_t) PSEUDO_HEADER_SIZE || + (size_t) r != PSEUDO_HEADER_SIZE + incoming->pathlen) { + pseudo_error("dropped malformed request (%ld bytes).\n", (long) r); + errno = EBADMSG; + return 0; + } + pseudo_debug(PDBGF_IPC, "got request, type %d, pathlen %d\n", incoming->type, (int) incoming->pathlen); + ((char *) incoming)[r] = '\0'; + return incoming; +} + /* duplicate a message -- currently totally unused */ pseudo_msg_t * pseudo_msg_dup(pseudo_msg_t *old) { diff --git a/pseudo_ipc.h b/pseudo_ipc.h index d945257..1fc20a9 100644 --- a/pseudo_ipc.h +++ b/pseudo_ipc.h @@ -26,6 +26,8 @@ typedef struct { unsigned int pathlen; int nlink; int deleting; + int client_id; + unsigned int client_serial; char path[]; } pseudo_msg_t; @@ -47,7 +49,13 @@ extern int pseudo_access_fopen(const char *); #define PSEUDO_HEADER_SIZE (offsetof(pseudo_msg_t, path)) +/* largest request sent over the datagram socket: two paths for rename, + * or a path plus an xattr name and value (at most 64KB) + */ +#define PSEUDO_MAX_REQUEST (256 * 1024) + extern pseudo_msg_t *pseudo_msg_receive(int fd); +extern pseudo_msg_t *pseudo_msg_receive_request(int fd); extern pseudo_msg_t *pseudo_msg_dup(pseudo_msg_t *); extern pseudo_msg_t *pseudo_msg_dupheader(pseudo_msg_t *); extern pseudo_msg_t *pseudo_msg_new(size_t, const char *); diff --git a/pseudo_server.c b/pseudo_server.c index 13caced..6109f67 100644 --- a/pseudo_server.c +++ b/pseudo_server.c @@ -41,6 +41,7 @@ typedef struct { pid_t pid; char *tag; char *program; + unsigned int serial; } pseudo_client_t; pseudo_client_t *clients; @@ -51,6 +52,19 @@ pseudo_client_t *clients; */ static int active_clients = 0, highest_client = 0, max_clients = 0; +/* Clients send all requests over one datagram socket, so the server + * handles them in the order they were sent, even across clients: once + * one client's send returns, its request is queued ahead of anything + * another client sends afterwards. Replies go back over each client's + * stream connection, which also carries the initial ping, shutdown + * requests, and tells us when the client goes away. Each connection + * gets a serial number so a request from a client that has since gone + * away is never answered on a reused client slot. + */ +static int request_fd = -1; +static unsigned int next_serial = 0; +#define REQUEST_EVENT ((uint64_t) -1) + #define LOOP_DELAY 2 #define DEFAULT_PSEUDO_SERVER_TIMEOUT 30 #define EPOLL_MAX_EVENTS 10 @@ -346,6 +360,34 @@ pseudo_server_start(int daemonize) { pseudo_error("couldn't listen on socket: %s\n", strerror(errno)); exit(PSEUDO_EXIT_SOCKET_LISTEN); } + + request_fd = socket(PF_UNIX, SOCK_DGRAM, 0); + if (request_fd < 0) { + pseudo_error("couldn't create request socket: %s\n", strerror(errno)); + exit(PSEUDO_EXIT_SOCKET_CREATE); + } + if (request_fd <= 2) { + newfd = fcntl(request_fd, F_DUPFD, 3); + if (newfd < 0) { + pseudo_error("couldn't dup request socket: %s\n", strerror(errno)); + exit(PSEUDO_EXIT_SOCKET_FD); + } + close(request_fd); + request_fd = newfd; + } + rc = unlink(PSEUDO_REQUEST_SOCKET); + if (rc == -1 && errno != ENOENT) { + pseudo_error("Can't unlink existing request socket: %s.\n", + strerror(errno)); + exit(PSEUDO_EXIT_SOCKET_UNLINK); + } + { + struct sockaddr_un req_sun = { .sun_family = AF_UNIX, .sun_path = PSEUDO_REQUEST_SOCKET }; + if (bind(request_fd, (struct sockaddr *) &req_sun, sizeof(req_sun)) == -1) { + pseudo_error("couldn't bind request socket: %s\n", strerror(errno)); + exit(PSEUDO_EXIT_SOCKET_BIND); + } + } rc = pseudo_server_write_pid(getpid()); if (rc != 0) { pseudo_warning("couldn't write pid file.\n"); @@ -384,6 +426,7 @@ open_client(int fd) { clients[i].pid = 0; clients[i].tag = NULL; clients[i].program = NULL; + clients[i].serial = ++next_serial; ++active_clients; if (i > highest_client) highest_client = i; @@ -407,6 +450,7 @@ open_client(int fd) { clients[max_clients].pid = 0; clients[max_clients].tag = NULL; clients[max_clients].program = NULL; + clients[max_clients].serial = ++next_serial; highest_client = max_clients; max_clients += 16; @@ -491,6 +535,11 @@ serve_client(int i) { } pseudo_debug(PDBGF_SERVER, "\n"); } + /* tell the client which slot to name in its requests */ + if (in->type == PSEUDO_MSG_PING) { + in->client_id = i; + in->client_serial = clients[i].serial; + } if (in->type == PSEUDO_MSG_SHUTDOWN && !clients[i].pid) { pseudo_debug(PDBGF_SERVER, "shutdown request from client %d [pid %d]\n", i, in->client); @@ -605,6 +654,93 @@ serve_client(int i) { return rc; } +/* Handle one request from the datagram socket, replying on the client's + * stream connection. + */ +static void +serve_request(pseudo_msg_t *in) { + struct timeval tv1, tv2; + char *response_path = 0; + size_t response_pathlen = -1; + int id = in->client_id; + int valid, rc; + + gettimeofday(&tv1, NULL); + ++messages; + valid = id > 0 && id <= highest_client && clients[id].fd != -1 && + clients[id].serial == in->client_serial; + if (!valid) { + pseudo_debug(PDBGF_SERVER, "request from departed client %d [pid %d]\n", + id, in->client); + } else if (in->client != clients[id].pid) { + pseudo_debug(PDBGF_SERVER, "uh-oh, expected pid %d for client %d, got %d\n", + (int) clients[id].pid, id, in->client); + } + switch (in->type) { + case PSEUDO_MSG_PING: + in->type = PSEUDO_MSG_ACK; + break; + case PSEUDO_MSG_OP: + case PSEUDO_MSG_FASTOP: + if (pseudo_server_response(in, valid ? clients[id].program : NULL, + valid ? clients[id].tag : NULL, + &response_path, &response_pathlen)) { + in->type = PSEUDO_MSG_NAK; + } else { + in->type = PSEUDO_MSG_ACK; + } + break; + default: + pdb_log_msg(SEVERITY_WARN, in, NULL, NULL, "invalid request type %d", in->type); + in->type = PSEUDO_MSG_NAK; + break; + } + if (valid) { + pseudo_debug(PDBGF_SERVER | PDBGF_VERBOSE, "response: %d (%s)\n", + in->result, pseudo_res_name(in->result)); + in->client = id; + in->pathlen = response_path ? response_pathlen : 0; + rc = pseudo_msg_send(clients[id].fd, in, in->pathlen, response_path); + if (rc != 0) { + pseudo_debug(PDBGF_SERVER, "failed to send response to client %d [%d]: %d (%s)\n", + id, (int) clients[id].pid, rc, strerror(errno)); + } else { + ++responses; + } + } + free(response_path); + + gettimeofday(&tv2, NULL); + message_time.tv_sec += (tv2.tv_sec - tv1.tv_sec); + message_time.tv_usec += (tv2.tv_usec - tv1.tv_usec); + if (message_time.tv_usec < 0) { + message_time.tv_usec += 1000000; + --message_time.tv_sec; + } else while (message_time.tv_usec > 1000000) { + message_time.tv_usec -= 1000000; + ++message_time.tv_sec; + } +} + +/* Handle up to max queued requests, returning how many were handled. */ +static int +serve_requests(int max) { + pseudo_msg_t *in; + int n; + + for (n = 0; n < max; ++n) { + in = pseudo_msg_receive_request(request_fd); + if (!in) { + /* a malformed datagram was discarded; keep going */ + if (errno == EBADMSG) + continue; + break; + } + serve_request(in); + } + return n; +} + /* get clients, handle messages, shut down. * This doesn't actually do any work, it just calls a ton of things which * do work. @@ -665,6 +801,12 @@ pseudo_server_loop(void) { pseudo_error("epoll_ctl() failed with listening socket.\n"); exit(PSEUDO_EXIT_EPOLL_CTL); } + ev.events = EPOLLIN; + ev.data.u64 = REQUEST_EVENT; + if (epoll_ctl(epollfd, EPOLL_CTL_ADD, request_fd, &ev) == -1) { + pseudo_error("epoll_ctl() failed with request socket.\n"); + exit(PSEUDO_EXIT_EPOLL_CTL); + } pdb_log_msg(SEVERITY_INFO, NULL, NULL, NULL, "server started (pid %d)", getpid()); @@ -693,7 +835,14 @@ pseudo_server_loop(void) { } else if (rc > 0) { loop_timeout = pseudo_server_timeout; for (i = 0; i < rc; ++i) { - int client_id = events[i].data.u64; + int client_id; + if (events[i].data.u64 == REQUEST_EVENT) { + serve_requests(64); + if (die_forcefully) + break; + continue; + } + client_id = events[i].data.u64; if (clients[client_id].fd == listen_fd) { if (!die_forcefully) { len = sizeof(client); @@ -780,6 +929,9 @@ pseudo_server_loop(void) { pseudo_info("done.\n"); } if (die_peacefully || die_forcefully) { + /* apply requests clients sent before they went away */ + while (serve_requests(64) == 64) + ; pseudo_debug(PDBGF_SERVER, "quitting.\n"); pseudo_debug(PDBGF_SERVER | PDBGF_BENCHMARK, "server %d exiting: handled %d messages in %.4f seconds\n", getpid(), messages, From patchwork Wed Nov 11 04:28:07 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Hatle X-Patchwork-Id: 103647 Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) by mx.groups.io with SMTP id smtpd.msgproc02-g2.2564.1794371289296305506 for ; Tue, 10 Nov 2026 20:28:09 -0800 Authentication-Results: mx.groups.io; dkim=none (message not signed); spf=pass (domain: kernel.crashing.org, ip: 63.228.1.57, mailfrom: mark.hatle@kernel.crashing.org) Received: from kernel.crashing.net (70-99-78-136.nuveramail.net [70.99.78.136] (may be forged)) by gate.crashing.org (8.18.1/8.18.1/Debian-2) with ESMTP id 6AB4S8mG2429215 for ; Tue, 10 Nov 2026 22:28:08 -0600 From: Mark Hatle To: yocto-patches@lists.yoctoproject.org Subject: [pseudo][PATCH 2/2] Replace FASTOP with requests that don't wait for a reply Date: Tue, 10 Nov 2026 22:28:07 -0600 Message-Id: <1794371287-31540-3-git-send-email-mark.hatle@kernel.crashing.org> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1794371287-31540-1-git-send-email-mark.hatle@kernel.crashing.org> References: <1794371287-31540-1-git-send-email-mark.hatle@kernel.crashing.org> List-Id: X-Webhook-Received: from 45-33-107-173.ip.linodeusercontent.com [45.33.107.173] by lin-tor-korg-lkml-alma10-1 with HTTPS for ; Wed, 11 Nov 2026 04:28:17 -0000 X-Groupsio-URL: https://lists.yoctoproject.org/g/yocto-patches/message/5297 MIME-Version: 1.0 From: Mark Hatle Most requests only tell the server about a change, such as chown, chmod, creat, rename or unlink, and the client ignores the reply, but it still waited for every one. FASTOP was meant to avoid that, but it sent requests over each client's own connection, so another process could overtake a change it had been told about, and it dropped errors. "Half-undo FASTOP" (449c234) fixed the errors by having the client wait for every reply again, leaving FASTOP identical to OP; the server's early-ACK path never ran because the message type had already been changed to ACK or NAK. Remove it, its op "wait" column and the dead server code. Since requests are now handled in the order they were sent, even across processes, send these as a new PSEUDO_MSG_ASYNC type without waiting. The ops are marked in a new "async" column in enums/op.in. The server only replies to an async request if the client must abort, for instance after a path mismatch. The client reads that reply ahead of the reply to its next synchronous request and aborts then. So that an abort isn't lost, a client with requests outstanding waits for them with a PING on the request socket before exec() and at exit; the exit check doesn't wait for the pseudo lock, which another thread may hold indefinitely. _exit() and _Exit() skip destructors, and are used by forked workers such as Python's multiprocessing, so wrap them to wait as well. PSEUDO_SYNC=1 makes every request wait, so the client aborts in the call that caused the problem. On a tar/chown/find/ls/rm workload of 40,000 files this takes the run from 21.8s to 14.3s. Add tests that changes made without waiting are seen by other processes told about them through a pipe or by reaping the process, and that an abort is still delivered, at the next synchronous request, _exit() or execvpe(), with and without PSEUDO_SYNC. AI-Generated: Uses GitHub Copilot Signed-off-by: Mark Hatle --- doc/overview | 3 +- doc/program_flow | 11 +++ doc/pseudo_ipc | 43 +++++++--- enums/msg_type.in | 2 +- enums/op.in | 50 +++++------ ports/common/guts/_Exit.c | 16 ++++ ports/common/guts/_exit.c | 16 ++++ ports/common/guts/execv.c | 4 + ports/common/guts/execve.c | 4 + ports/common/guts/execveat.c | 4 + ports/common/guts/execvp.c | 4 + ports/common/guts/execvpe.c | 4 + ports/common/guts/fexecve.c | 4 + ports/common/pseudo_wrappers.c | 34 ++++++++ ports/common/wrapfuncs.in | 2 + pseudo.1 | 23 ++++- pseudo.c | 2 +- pseudo_client.c | 72 +++++++++++++++- pseudo_client.h | 2 + pseudo_server.c | 42 ++++----- pseudo_util.c | 1 + pseudo_wrappers.c | 43 ++++++++++ test/test-async-abort.c | 44 ++++++++++ test/test-async-abort.sh | 40 +++++++++ test/test-async-order.c | 188 +++++++++++++++++++++++++++++++++++++++++ test/test-async-order.sh | 23 +++++ 26 files changed, 606 insertions(+), 75 deletions(-) create mode 100644 ports/common/guts/_Exit.c create mode 100644 ports/common/guts/_exit.c create mode 100644 test/test-async-abort.c create mode 100755 test/test-async-abort.sh create mode 100644 test/test-async-order.c create mode 100755 test/test-async-order.sh diff --git a/doc/overview b/doc/overview index c9497e5..7b7ab1e 100644 --- a/doc/overview +++ b/doc/overview @@ -47,7 +47,8 @@ client can restart it.) 4. The server code. This code is fairly simple; all it does is maintain the database of file information. Operations consist either of a request for information (e.g., a stat(2) call) or notification of a change. The -server sends back failure or success notices. +server sends back failure or success notices for requests for information; +notifications of a change are sent without waiting for a reply. As a fairly typical example, the progress of a stat(2) call is: diff --git a/doc/program_flow b/doc/program_flow index 5fbdb5d..174f443 100644 --- a/doc/program_flow +++ b/doc/program_flow @@ -14,6 +14,8 @@ libpseudo execution flow: setup pseudo_functions setup log files (PSEUDO_DEBUG_FILE, PSEUDO_EVLOG_FILE) pseudo_client.c: pseudo_init_client() + wait for this process's asynchronous requests + setup PSEUDO_SYNC setup PSEUDO_DISABLED setup pseudo_localstate_dir_fd setup PSEUDO_NOSYMLINKEXP @@ -30,6 +32,7 @@ libpseudo execution flow: pseudo_setupenv() if PSEUDO_UNLOAD pseudo_dropenv() + pseudo_client_flush() real_exec*() fork() @@ -77,6 +80,14 @@ libpseudo execution flow: unmask signals (if they were blocked above) return rc + # on exit # + + pseudo_wrappers.c: (destructor) _libpseudo_fini(), or _exit()/_Exit() + pseudo_exit_flush() + if this process has asynchronous requests pending, + and the lock is free (or already ours) + pseudo_client_flush() + pseudo execution flow: pseudo.c: main() pseudo_util.c: pseudo_init_util() diff --git a/doc/pseudo_ipc b/doc/pseudo_ipc index e4ec750..8186cd6 100644 --- a/doc/pseudo_ipc +++ b/doc/pseudo_ipc @@ -20,8 +20,10 @@ changes a file and then tells another process about it (through a pipe, by exiting, or any other way), the server has already queued the change by the time the second process can ask about the file. -It also means the server reads each request with a single recv(), -rather than a header and then a path. +That is what allows requests that need no reply to be sent without +waiting for the server (see ASYNC below). It also means the server +reads each request with a single recv(), rather than a header and then +a path. typedef struct { pseudo_msg_type_t type; @@ -53,26 +55,41 @@ All messages contain every field up to, but not including, path. If pathlen is not zero, an additional pathlen bytes containing path are provided; path is null-terminated. -Every message from client should get a response from server. The server -usually sends no path back; the exceptions are replies to xattr requests -(attribute values or name lists) and the list of client PIDs in a refused -SHUTDOWN request, described below. Note that -all server responses will in general share a single message object, +Every message from the client gets a response from the server, except +ASYNC requests. The server usually sends no path back; the exceptions +are replies to xattr requests (attribute values or name lists) and the +list of client PIDs in a refused SHUTDOWN request, described below. +Note that all server responses will in general share a single message object, and future operations may cause that object to be reallocated; the same goes for messages received by the server. Basically, pseudo_msg_receive is not thread-safe; this is part of (but not all of) the reason that there's mutex stuff in the wrappers. (The other part is the "antimagic" being able to blow things up.) -type is one of PING, OP, FASTOP, SHUTDOWN, ACK, or NAK. The client -only sends PING, OP, or FASTOP. FASTOP is used for operations whose result -the client doesn't need, but the server still replies with ACK (or NAK on -failure) and the client still waits for that reply. +type is one of PING, OP, SHUTDOWN, ACK, NAK, or ASYNC. The client +sends PING, OP and ASYNC, and the server replies to PING and OP with ACK +(or NAK on failure); the client waits for that reply. A PING on the stream socket registers the client: the reply gives it the client_id and client_serial to put in its requests. A PING on the request socket is answered once every request the client sent before it -has been handled. +has been handled; the client uses this to wait for its ASYNC requests. + +ASYNC is used instead of OP for operations whose result the client +doesn't use: chmod, chown, creat, link, mkdir, rename, symlink, unlink, +did-unlink, cancel-unlink and set-xattr, and open and exec when the +server is logging. These are marked in enums/op.in. The client doesn't +wait for them, and the server only replies if the client must abort +(RESULT_ABORT, for instance after a path mismatch). That reply has type +ASYNC, and arrives on the stream connection ahead of the reply to the +client's next OP or PING, which is where the client reads it and aborts. +To make sure an abort isn't lost, a client that has sent ASYNC requests +sends a PING on the request socket and waits for the reply before +exec() and when it exits, including through _exit(). A client that is +killed doesn't, but the server still applies its requests. + +Setting PSEUDO_SYNC in a client's environment to a value other than 0 +makes it send OP instead of ASYNC, waiting for every request. When run with '-S', the pseudo program runs as a client, sending a SHUTDOWN message to a server -- but only if @@ -85,7 +102,7 @@ reduces the timeout to 3 seconds, so it shuts down shortly after the last client goes away rather than waiting for the full timeout period.) result is the result of a particular operation. It applies only in replies -to OP messages. +to OP and ASYNC messages. client should be the client's PID on send, and the server's client number for that client on response. (The response isn't checked, and this is just a diff --git a/enums/msg_type.in b/enums/msg_type.in index 578d571..8435599 100644 --- a/enums/msg_type.in +++ b/enums/msg_type.in @@ -4,4 +4,4 @@ shutdown op ack nak -fastop +async diff --git a/enums/op.in b/enums/op.in index 5854b74..f9b5e22 100644 --- a/enums/op.in +++ b/enums/op.in @@ -1,30 +1,30 @@ -op: OP; int wait = 0 +op: OP; int async = 0 chdir, 0 -chmod, 0 -chown, 0 +chmod, 1 +chown, 1 chroot, 0 close, 0 -creat, 0 +creat, 1 dup, 0 -fchmod, 0 -fchown, 0 -fstat, 1 -link, 0 -mkdir, 0 -mknod, 1 -open, 0 -rename, 0 -stat, 1 -unlink, 0 -symlink, 0 -exec, 0 -may-unlink, 1 -did-unlink, 0 -cancel-unlink, 0 -get-xattr, 1 -list-xattr, 1 -remove-xattr, 1 -set-xattr, 0 -create-xattr, 1 -replace-xattr, 1 +fchmod, 1 +fchown, 1 +fstat, 0 +link, 1 +mkdir, 1 +mknod, 0 +open, 1 +rename, 1 +stat, 0 +unlink, 1 +symlink, 1 +exec, 1 +may-unlink, 0 +did-unlink, 1 +cancel-unlink, 1 +get-xattr, 0 +list-xattr, 0 +remove-xattr, 0 +set-xattr, 1 +create-xattr, 0 +replace-xattr, 0 close-range, 0 diff --git a/ports/common/guts/_Exit.c b/ports/common/guts/_Exit.c new file mode 100644 index 0000000..5c93d2d --- /dev/null +++ b/ports/common/guts/_Exit.c @@ -0,0 +1,16 @@ +/* + * SPDX-License-Identifier: LGPL-2.1-only + * + * void _Exit(int status) + */ + + /* NOTE THAT THIS IS NEVER USED! + * The hand-written _Exit() in ../pseudo_wrappers.c flushes + * asynchronous requests and exits directly. + */ + + real__Exit(status); + +/* return; + * } + */ diff --git a/ports/common/guts/_exit.c b/ports/common/guts/_exit.c new file mode 100644 index 0000000..06f2520 --- /dev/null +++ b/ports/common/guts/_exit.c @@ -0,0 +1,16 @@ +/* + * SPDX-License-Identifier: LGPL-2.1-only + * + * void _exit(int status) + */ + + /* NOTE THAT THIS IS NEVER USED! + * The hand-written _exit() in ../pseudo_wrappers.c flushes + * asynchronous requests and exits directly. + */ + + real__exit(status); + +/* return; + * } + */ diff --git a/ports/common/guts/execv.c b/ports/common/guts/execv.c index d4031b3..dc3540b 100644 --- a/ports/common/guts/execv.c +++ b/ports/common/guts/execv.c @@ -33,6 +33,10 @@ /* if exec() fails, we may end up taking signals unexpectedly... * not much we can do about that. */ + /* exec() skips destructors, so collect errors for requests + * sent without waiting now + */ + pseudo_client_flush(); sigprocmask(SIG_SETMASK, &pseudo_saved_sigmask, NULL); rc = real_execv(file, argv); diff --git a/ports/common/guts/execve.c b/ports/common/guts/execve.c index de2f1d4..29cb637 100644 --- a/ports/common/guts/execve.c +++ b/ports/common/guts/execve.c @@ -27,6 +27,10 @@ /* if exec() fails, we may end up taking signals unexpectedly... * not much we can do about that. */ + /* exec() skips destructors, so collect errors for requests + * sent without waiting now + */ + pseudo_client_flush(); sigprocmask(SIG_SETMASK, &pseudo_saved_sigmask, NULL); rc = real_execve(file, argv, new_environ); diff --git a/ports/common/guts/execveat.c b/ports/common/guts/execveat.c index 10e0d52..b591dc4 100644 --- a/ports/common/guts/execveat.c +++ b/ports/common/guts/execveat.c @@ -19,6 +19,10 @@ if (pseudo_has_unload(new_environ)) new_environ = pseudo_dropenvp(new_environ); + /* exec() skips destructors, so collect errors for requests + * sent without waiting now + */ + pseudo_client_flush(); sigprocmask(SIG_SETMASK, &pseudo_saved_sigmask, NULL); rc = real_execveat(dirfd, path, argv, new_environ, flags); diff --git a/ports/common/guts/execvp.c b/ports/common/guts/execvp.c index ffc94be..2d0c8f2 100644 --- a/ports/common/guts/execvp.c +++ b/ports/common/guts/execvp.c @@ -33,6 +33,10 @@ /* if exec() fails, we may end up taking signals unexpectedly... * not much we can do about that. */ + /* exec() skips destructors, so collect errors for requests + * sent without waiting now + */ + pseudo_client_flush(); sigprocmask(SIG_SETMASK, &pseudo_saved_sigmask, NULL); rc = real_execvp(file, argv); diff --git a/ports/common/guts/execvpe.c b/ports/common/guts/execvpe.c index 37e4927..5fc544b 100644 --- a/ports/common/guts/execvpe.c +++ b/ports/common/guts/execvpe.c @@ -19,6 +19,10 @@ if (pseudo_has_unload(new_environ)) new_environ = pseudo_dropenvp(new_environ); + /* exec() skips destructors, so collect errors for requests + * sent without waiting now + */ + pseudo_client_flush(); sigprocmask(SIG_SETMASK, &pseudo_saved_sigmask, NULL); rc = real_execvpe(file, argv, new_environ); diff --git a/ports/common/guts/fexecve.c b/ports/common/guts/fexecve.c index a0162d3..8e64ae0 100644 --- a/ports/common/guts/fexecve.c +++ b/ports/common/guts/fexecve.c @@ -11,6 +11,10 @@ if (pseudo_has_unload(new_environ)) new_environ = pseudo_dropenvp(new_environ); + /* exec() skips destructors, so collect errors for requests + * sent without waiting now + */ + pseudo_client_flush(); sigprocmask(SIG_SETMASK, &pseudo_saved_sigmask, NULL); rc = real_fexecve(fd, argv, new_environ); diff --git a/ports/common/pseudo_wrappers.c b/ports/common/pseudo_wrappers.c index 2ad0814..a391b48 100644 --- a/ports/common/pseudo_wrappers.c +++ b/ports/common/pseudo_wrappers.c @@ -445,6 +445,29 @@ execveat(int dirfd, const char *path, char *const *argv, char *const *envp, int return rc; } +/* _exit() skips destructors, so collect errors for requests sent + * without waiting here instead. + */ +void +_exit(int status) { + pseudo_exit_flush(); + if (real__exit) + real__exit(status); + syscall(SYS_exit_group, status); + for (;;) + ; +} + +void +_Exit(int status) { + pseudo_exit_flush(); + if (real__Exit) + real__Exit(status); + syscall(SYS_exit_group, status); + for (;;) + ; +} + /* no profiling in fork because it wouldn't work anyway * half the time */ @@ -543,6 +566,17 @@ wrap_execveat(int dirfd, const char *path, char *const *argv, char *const *envp, return rc; } +/* not used, the hand-written wrappers flush and exit directly */ +static void +wrap__exit(int status) { + (void) status; +} + +static void +wrap__Exit(int status) { + (void) status; +} + static int wrap_fork(void) { int rc = -1; diff --git a/ports/common/wrapfuncs.in b/ports/common/wrapfuncs.in index 9b01bf2..10a5fae 100644 --- a/ports/common/wrapfuncs.in +++ b/ports/common/wrapfuncs.in @@ -10,3 +10,5 @@ int posix_spawnp(pid_t *pid, const char *file, const posix_spawn_file_actions_t int execvpe(const char *file, char *const *argv, char *const *envp); /* hand_wrapped=1 */ int fexecve(int fd, char *const *argv, char *const *envp); /* hand_wrapped=1 */ int execveat(int dirfd, const char *path, char *const *argv, char *const *envp, int flags); /* hand_wrapped=1 */ +void _exit(int status); /* hand_wrapped=1 */ +void _Exit(int status); /* hand_wrapped=1 */ diff --git a/pseudo.1 b/pseudo.1 index 2ccf6bd..2475631 100644 --- a/pseudo.1 +++ b/pseudo.1 @@ -798,6 +798,21 @@ suppresses informational messages or warnings; errors are always shown. The default is .IR info,warn,error . .TP 8 +.B PSEUDO_SYNC +By default, requests whose result the client doesn't use, such as +.IR chown(2) +or +.IR chmod(2) , +are sent to the server without waiting for it to handle them. The +server still handles every request in the order clients sent it, so +other processes always see the change. If the server asks the client +to abort, for instance after a path mismatch, the client aborts at its +next request that does wait for a reply, or before +.IR exec(3) +or exiting. If this variable is set to a value other than 0, the +client waits for every request, so it aborts in the call that caused +the problem. This is slower, but can help when debugging. +.TP 8 .BR PSEUDO_UIDS ,\ PSEUDO_GIDS These variables are used internally to pass information about the current emulated user and group identity from one process to another. @@ -855,9 +870,11 @@ or debugged in that context. Filesystem performance is noticably worse under .I pseudo than it is otherwise. This is probably because nearly every operation -(other than reads and writes) involves at least one round-trip network -communication with the server, and probably some kind of database -activity. +(other than reads and writes) involves a message to the server, and +probably some kind of database activity. Operations that ask about a +file wait for the server's reply; ones that only report a change do not +(see +.BR PSEUDO_SYNC ). .SH SECURITY .I pseudo diff --git a/pseudo.c b/pseudo.c index 26c7a36..70f439d 100644 --- a/pseudo.c +++ b/pseudo.c @@ -1087,7 +1087,7 @@ pseudo_server_response(pseudo_msg_t *msg, const char *program, const char *tag, return 0; break; case PSEUDO_MSG_OP: - case PSEUDO_MSG_FASTOP: + case PSEUDO_MSG_ASYNC: return pseudo_op(msg, program, tag, response_path, response_len); break; case PSEUDO_MSG_ACK: /* FALLTHROUGH */ diff --git a/pseudo_client.c b/pseudo_client.c index c5b960f..79e1e29 100644 --- a/pseudo_client.c +++ b/pseudo_client.c @@ -53,6 +53,13 @@ static int connect_fd = -1; static int request_fd = -1; static int client_id = 0; static unsigned int client_serial = 0; +/* requests that need no reply are sent without waiting. async_pending + * is set while some may not have been handled yet, so their errors are + * collected before exec() or exit(). + */ +static int async_pending = 0; +static pid_t async_pid = 0; +static int pseudo_sync_only = 0; static int server_pid = 0; int pseudo_localstate_dir_fd = -1; static int pseudo_pathlock_fd = -1; @@ -454,8 +461,17 @@ pseudo_init_client(void) { int need_free = 0; pseudo_antimagic(); + /* collect errors for requests this process sent; a forked child + * leaves its parent's to the parent + */ + pseudo_client_flush(); + async_pending = 0; pseudo_new_pid(); client_disconnect(); + + env = pseudo_get_value("PSEUDO_SYNC"); + pseudo_sync_only = env && *env && strcmp(env, "0"); + free(env); #ifdef PSEUDO_PROFILING if (pseudo_profile_fd > -1) { close(pseudo_profile_fd); @@ -1470,6 +1486,15 @@ pseudo_client_send(pseudo_msg_t *msg, size_t len, const char *path, int tries) { return -1; } +/* Send a request that needs no reply, without waiting for the server. */ +static void +pseudo_client_send_async(pseudo_msg_t *msg, size_t len, const char *path) { + msg->type = PSEUDO_MSG_ASYNC; + pseudo_client_send(msg, len, path, 0); + async_pending = 1; + async_pid = getpid(); +} + static pseudo_msg_t * pseudo_client_request(pseudo_msg_t *msg, size_t len, const char *path) { pseudo_msg_t *response = 0; @@ -1488,7 +1513,16 @@ pseudo_client_request(pseudo_msg_t *msg, size_t len, const char *path) { for (; tries < PSEUDO_RETRIES; ++tries) { tries = pseudo_client_send(msg, len, path, tries); pseudo_debug(PDBGF_CLIENT | PDBGF_VERBOSE, "sent!\n"); - response = pseudo_msg_receive(connect_fd); + /* replies come back in order, so any errors for earlier + * asynchronous requests arrive first + */ + while ((response = pseudo_msg_receive(connect_fd)) != NULL && + response->type == PSEUDO_MSG_ASYNC) { + pseudo_debug(PDBGF_CLIENT, "error for asynchronous %s: %s\n", + pseudo_op_name(response->op), pseudo_res_name(response->result)); + if (response->result == RESULT_ABORT) + pseudo_client_abort(); + } if (!response) { pseudo_debug(PDBGF_CLIENT, "expected response did not occur; retrying\n"); /* the connection may be gone even though the request @@ -1496,6 +1530,8 @@ pseudo_client_request(pseudo_msg_t *msg, size_t len, const char *path) { */ pseudo_client_setup(); } else { + /* everything sent before this request has been handled */ + async_pending = 0; if (response->type != PSEUDO_MSG_ACK) { pseudo_debug(PDBGF_CLIENT, "got non-ack response %d\n", response->type); return 0; @@ -1511,6 +1547,34 @@ pseudo_client_request(pseudo_msg_t *msg, size_t len, const char *path) { return 0; } +/* Whether this process has asynchronous requests the server may not + * have handled yet. + */ +int +pseudo_client_flush_needed(void) { + return async_pending && async_pid == getpid() && request_fd != -1; +} + +/* Wait until the server has handled every asynchronous request this + * process sent, so any abort they caused happens before exec() or exit(). + */ +void +pseudo_client_flush(void) { + pseudo_msg_t msg; + + if (!pseudo_client_flush_needed()) + return; + pseudo_antimagic(); + memset(&msg, 0, sizeof(msg)); + msg.type = PSEUDO_MSG_PING; + msg.op = OP_NONE; + msg.client = getpid(); + pseudo_debug(PDBGF_CLIENT | PDBGF_VERBOSE, "flushing asynchronous requests\n"); + pseudo_client_request(&msg, 0, NULL); + async_pending = 0; + pseudo_magic(); +} + int pseudo_client_shutdown(int wait_on_socket) { pseudo_msg_t msg; @@ -2299,8 +2363,6 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path #ifdef PSEUDO_PROFILING struct timeval tv1_ipc, tv2_ipc; #endif - if (!pseudo_op_wait(msg.op)) - msg.type = PSEUDO_MSG_FASTOP; pseudo_debug(PDBGF_CLIENT | PDBGF_VERBOSE, "sending request [ino %llu]\n", (unsigned long long) msg.ino); #ifdef PSEUDO_PROFILING gettimeofday(&tv1_ipc, NULL); @@ -2308,6 +2370,8 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path if (pseudo_local_only) { /* disable server */ result = NULL; + } else if (!pseudo_sync_only && pseudo_op_async(msg.op)) { + pseudo_client_send_async(&msg, pathlen, path); } else { result = pseudo_client_request(&msg, pathlen, path); } @@ -2334,7 +2398,7 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path (int) result->uid, (int) result->gid); } - } else if (msg.type != PSEUDO_MSG_FASTOP) { + } else if (msg.type != PSEUDO_MSG_ASYNC) { pseudo_debug(PDBGF_OP, "(%d) no answer", getpid()); } } else if (!result) { diff --git a/pseudo_client.h b/pseudo_client.h index ef959e6..b9a8405 100644 --- a/pseudo_client.h +++ b/pseudo_client.h @@ -19,6 +19,8 @@ extern void pseudo_magic(void); extern void pseudo_client_touchuid(void); extern void pseudo_client_touchgid(void); extern int pseudo_client_shutdown(int); +extern void pseudo_client_flush(void); +extern int pseudo_client_flush_needed(void); extern int pseudo_fd(int fd, int how); extern int pseudo_client_path_lock(int shared); extern void pseudo_client_path_unlock(int fd); diff --git a/pseudo_server.c b/pseudo_server.c index 6109f67..7016adc 100644 --- a/pseudo_server.c +++ b/pseudo_server.c @@ -508,7 +508,6 @@ serve_client(int i) { if (in) { char *response_path = 0; size_t response_pathlen; - int send_response = 1; pseudo_debug(PDBGF_SERVER | PDBGF_VERBOSE, "got a message (%d): %s\n", in->type, (in->pathlen ? in->path : "")); /* handle incoming ping */ if (in->type == PSEUDO_MSG_PING && !clients[i].pid) { @@ -570,20 +569,6 @@ serve_client(int i) { } else { in->pathlen = 0; } - if (in->type == PSEUDO_MSG_FASTOP) { - /* respond instantly */ - send_response = 0; - int t_type = in->type; - int t_pathlen = in->pathlen; - in->type = PSEUDO_MSG_ACK; - in->pathlen = 0; - if ((rc = pseudo_msg_send(clients[i].fd, in, in->pathlen, response_path)) != 0) { - pseudo_debug(PDBGF_SERVER, "failed to send response to client %d [%d]: %d (%s)\n", - i, (int) clients[i].pid, rc, strerror(errno)); - } - in->type = t_type; - in->pathlen = t_pathlen; - } } else { /* the server's listen fd is "a client", and * so is the program connecting to request a shutdown. @@ -621,13 +606,9 @@ serve_client(int i) { die_peacefully = 1; } } - if (send_response) { - if ((rc = pseudo_msg_send(clients[i].fd, in, in->pathlen, response_path)) != 0) { - pseudo_debug(PDBGF_SERVER, "failed to send response to client %d [%d]: %d (%s)\n", - i, (int) clients[i].pid, rc, strerror(errno)); - } - } else { - rc = 1; + if ((rc = pseudo_msg_send(clients[i].fd, in, in->pathlen, response_path)) != 0) { + pseudo_debug(PDBGF_SERVER, "failed to send response to client %d [%d]: %d (%s)\n", + i, (int) clients[i].pid, rc, strerror(errno)); } free(response_path); } else { @@ -654,8 +635,10 @@ serve_client(int i) { return rc; } -/* Handle one request from the datagram socket, replying on the client's - * stream connection. +/* Handle one request from the datagram socket. Synchronous requests + * (and PING, which a client uses to wait until everything it sent so far + * has been handled) get a reply on the client's stream connection. + * Asynchronous ones only get a reply if the client must abort. */ static void serve_request(pseudo_msg_t *in) { @@ -663,7 +646,7 @@ serve_request(pseudo_msg_t *in) { char *response_path = 0; size_t response_pathlen = -1; int id = in->client_id; - int valid, rc; + int valid, reply = 1, rc; gettimeofday(&tv1, NULL); ++messages; @@ -681,7 +664,6 @@ serve_request(pseudo_msg_t *in) { in->type = PSEUDO_MSG_ACK; break; case PSEUDO_MSG_OP: - case PSEUDO_MSG_FASTOP: if (pseudo_server_response(in, valid ? clients[id].program : NULL, valid ? clients[id].tag : NULL, &response_path, &response_pathlen)) { @@ -690,12 +672,18 @@ serve_request(pseudo_msg_t *in) { in->type = PSEUDO_MSG_ACK; } break; + case PSEUDO_MSG_ASYNC: + pseudo_server_response(in, valid ? clients[id].program : NULL, + valid ? clients[id].tag : NULL, + &response_path, &response_pathlen); + reply = (in->result == RESULT_ABORT); + break; default: pdb_log_msg(SEVERITY_WARN, in, NULL, NULL, "invalid request type %d", in->type); in->type = PSEUDO_MSG_NAK; break; } - if (valid) { + if (reply && valid) { pseudo_debug(PDBGF_SERVER | PDBGF_VERBOSE, "response: %d (%s)\n", in->result, pseudo_res_name(in->result)); in->client = id; diff --git a/pseudo_util.c b/pseudo_util.c index d9312d0..4253eaa 100644 --- a/pseudo_util.c +++ b/pseudo_util.c @@ -58,6 +58,7 @@ static struct pseudo_variables pseudo_env[] = { { "PSEUDO_DISABLED", 15, NULL }, { "PSEUDO_UNLOAD", 13, NULL }, { "PSEUDO_ALLOW_FSYNC", 18, NULL }, + { "PSEUDO_SYNC", 11, NULL }, #ifdef PSEUDO_PROFILING { "PSEUDO_PROFILE_PATH", 19, NULL }, #endif diff --git a/pseudo_wrappers.c b/pseudo_wrappers.c index 13d7ede..2b1b39c 100644 --- a/pseudo_wrappers.c +++ b/pseudo_wrappers.c @@ -57,8 +57,10 @@ static pthread_mutex_t pseudo_mutex = PTHREAD_MUTEX_INITIALIZER; static pthread_t pseudo_mutex_holder; static int pseudo_mutex_recursion = 0; static int pseudo_getlock(void); +static int pseudo_trylock(void); static void pseudo_droplock(void); static int pseudo_lock_held(void); +static void pseudo_exit_flush(void); static size_t pseudo_dechroot(char *, size_t); static void pseudo_sigblock(sigset_t *); @@ -67,6 +69,7 @@ static sigset_t pseudo_saved_sigmask; /* Constructor only exists in libpseudo */ static void _libpseudo_init(void) __attribute__ ((constructor)); +static void _libpseudo_fini(void) __attribute__ ((destructor)); static int _libpseudo_initted = 0; @@ -119,6 +122,32 @@ _libpseudo_init(void) { pseudo_droplock(); } +/* Requests that need no reply are sent without waiting, so wait at exit + * for the server to handle them: if one of them makes the server ask the + * client to abort, that happens before the process is gone. Another + * thread may be blocked indefinitely inside a wrapper, holding the lock, + * so don't wait for the lock; that thread's next request will collect + * any abort if it gets one. + */ +static void +pseudo_exit_flush(void) { + sigset_t saved; + + if (!_libpseudo_initted || pseudo_disabled || !pseudo_client_flush_needed()) + return; + pseudo_sigblock(&saved); + if (pseudo_trylock() == 0) { + pseudo_client_flush(); + pseudo_droplock(); + } + sigprocmask(SIG_SETMASK, &saved, NULL); +} + +static void +_libpseudo_fini(void) { + pseudo_exit_flush(); +} + void pseudo_reinit_libpseudo(void) { _libpseudo_init(); @@ -228,6 +257,20 @@ pseudo_getlock(void) { } static int +pseudo_trylock(void) { + if (pthread_equal(pseudo_mutex_holder, pthread_self())) { + ++pseudo_mutex_recursion; + return 0; + } + if (pthread_mutex_trylock(&pseudo_mutex) == 0) { + pseudo_mutex_recursion = 1; + pseudo_mutex_holder = pthread_self(); + return 0; + } + return -1; +} + +static int pseudo_lock_held(void) { return pthread_equal(pseudo_mutex_holder, pthread_self()); } diff --git a/test/test-async-abort.c b/test/test-async-abort.c new file mode 100644 index 0000000..d0af068 --- /dev/null +++ b/test/test-async-abort.c @@ -0,0 +1,44 @@ +/* + * SPDX-License-Identifier: LGPL-2.1-only + * + * chown() a file whose database entry still has another name, which makes + * the server tell the client to abort. Prints "returned" if chown() + * returns, then does one of the following, which must deliver the abort + * if chown() didn't: + * stat a synchronous request + * _exit leave without running destructors + * execvpe run another program + * + * usage: test-async-abort + */ +#define _GNU_SOURCE +#include +#include +#include +#include + +int +main(int argc, char **argv) { + struct stat st; + + if (argc != 3) { + fprintf(stderr, "usage: %s \n", argv[0]); + return 2; + } + if (chown(argv[1], 1234, 5678) == -1) + perror("chown"); + printf("returned\n"); + fflush(stdout); + if (!strcmp(argv[2], "_exit")) { + _exit(0); + } else if (!strcmp(argv[2], "execvpe")) { + char *args[] = { "true", NULL }; + char *env[] = { NULL }; + execvpe("true", args, env); + perror("execvpe"); + return 2; + } + (void) stat(".", &st); + printf("survived\n"); + return 0; +} diff --git a/test/test-async-abort.sh b/test/test-async-abort.sh new file mode 100755 index 0000000..53cef39 --- /dev/null +++ b/test/test-async-abort.sh @@ -0,0 +1,40 @@ +#!/bin/bash +# +# SPDX-License-Identifier: LGPL-2.1-only +# +# A chown() whose database entry has another name makes the server tell +# the client to abort. chown() is sent without waiting, so the client +# must still abort: at its next request that waits for a reply, or when +# it leaves with _exit() or execs another program. With PSEUDO_SYNC set, +# it must abort inside chown() itself. +# +# The temporary tree has to live below $PWD so that pseudo tracks it. + +tmpdir="$(mktemp -d -p "$PWD" async_abort_XXXXXX)" +trap "rm -rf '$tmpdir'" EXIT TERM INT + +rc=0 +check() { + local sync="$1" how="$2" expect="$3" out status + : > "$tmpdir/a" + # rename it behind pseudo's back, so the database still says "a" + PSEUDO_DISABLED=1 mv "$tmpdir/a" "$tmpdir/b" + out=$(PSEUDO_SYNC=$sync ./test/test-async-abort "$tmpdir/b" "$how" 2>/dev/null) + status=$? + # put the name back so the database entry can be removed cleanly + PSEUDO_DISABLED=1 mv "$tmpdir/b" "$tmpdir/a" + rm -f "$tmpdir/a" + if [ $status -ne 134 ]; then + echo "FAILED: $how: expected abort (PSEUDO_SYNC=${sync:-unset}), got status $status" + rc=1 + elif [ "$out" != "$expect" ]; then + echo "FAILED: $how: PSEUDO_SYNC=${sync:-unset}: expected output '$expect', got '$out'" + rc=1 + fi +} + +for how in stat _exit execvpe; do + check "" $how "returned" +done +check 1 stat "" +exit $rc diff --git a/test/test-async-order.c b/test/test-async-order.c new file mode 100644 index 0000000..72d16b0 --- /dev/null +++ b/test/test-async-order.c @@ -0,0 +1,188 @@ +/* + * SPDX-License-Identifier: LGPL-2.1-only + * + * chown() and chmod() are sent to the server without waiting for a reply. + * Check that another process never sees the old ownership or mode once it + * has been told, through a pipe or by the process exiting, that the change + * was made. + * + * usage: test-async-order + */ +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include + +static char *dir; +static int count; +#define WRITERS 8 + +static void +name(char *buf, size_t len, int i) { + snprintf(buf, len, "%s/f%d", dir, i); +} + +static mode_t +mode_for(int i) { + return (i % 2) ? 0640 : 0604; +} + +static int +check(int i, int want_mode) { + char path[4096]; + struct stat st; + + name(path, sizeof(path), i); + if (stat(path, &st) == -1) { + fprintf(stderr, "stat %s: %s\n", path, strerror(errno)); + return 1; + } + if (st.st_uid != (uid_t) (1000 + i) || st.st_gid != (gid_t) (2000 + i) || + (want_mode && (st.st_mode & 07777) != mode_for(i))) { + fprintf(stderr, "%s: got %d:%d mode %o, expected %d:%d mode %o\n", + path, (int) st.st_uid, (int) st.st_gid, + (unsigned) (st.st_mode & 07777), 1000 + i, 2000 + i, + want_mode ? (unsigned) mode_for(i) : 0); + return 1; + } + return 0; +} + +static void +chown_one(int i) { + char path[4096]; + + name(path, sizeof(path), i); + if (chown(path, 1000 + i, 2000 + i) == -1) { + perror("chown"); + _exit(2); + } +} + +/* Each change is handed on through a pipe to a process that is already + * running: chown in one process, chmod in a second, stat in a third. + */ +static int +pipe_chain(void) { + int p12[2], p23[2]; + pid_t c2, c3; + int i, bad = 0, status; + + if (pipe(p12) == -1 || pipe(p23) == -1) { + perror("pipe"); + return 2; + } + c2 = fork(); + if (c2 == 0) { + char path[4096]; + close(p12[1]); + close(p23[0]); + while (read(p12[0], &i, sizeof(i)) == sizeof(i)) { + name(path, sizeof(path), i); + if (chmod(path, mode_for(i)) == -1) { + perror("chmod"); + _exit(2); + } + if (write(p23[1], &i, sizeof(i)) != sizeof(i)) + _exit(2); + } + _exit(0); + } + c3 = fork(); + if (c3 == 0) { + close(p12[0]); + close(p12[1]); + close(p23[1]); + while (read(p23[0], &i, sizeof(i)) == sizeof(i)) + bad += check(i, 1); + _exit(bad ? 1 : 0); + } + close(p12[0]); + close(p23[0]); + close(p23[1]); + /* send all the changes in a burst before handing them on, newest + * first, so the others ask about changes still queued behind + * older ones + */ + for (i = 0; i < count; ++i) + chown_one(i); + for (i = count - 1; i >= 0; --i) { + if (write(p12[1], &i, sizeof(i)) != sizeof(i)) { + perror("write"); + return 2; + } + } + close(p12[1]); + if (waitpid(c2, &status, 0) == -1 || !WIFEXITED(status) || WEXITSTATUS(status)) + bad = 1; + if (waitpid(c3, &status, 0) == -1 || !WIFEXITED(status) || WEXITSTATUS(status)) + bad = 1; + return bad; +} + +/* Several children change their files at once, so the server has a + * backlog from each, and leave with _exit(), which skips the exit-time + * flush. The parent checks, newest change first, as soon as they have + * all been reaped. + */ +static int +exit_handoff(void) { + pid_t c[WRITERS]; + int i, w, bad = 0, status; + + for (w = 0; w < WRITERS; ++w) { + c[w] = fork(); + if (c[w] == 0) { + for (i = 0; i < count; ++i) + chown_one(count * (w + 1) + i); + _exit(0); + } + } + for (w = 0; w < WRITERS; ++w) { + if (waitpid(c[w], &status, 0) == -1 || !WIFEXITED(status) || WEXITSTATUS(status)) + bad = 2; + } + if (bad) + return bad; + for (i = count * (WRITERS + 1) - 1; i >= count; --i) + bad += check(i, 0); + return bad ? 1 : 0; +} + +int +main(int argc, char **argv) { + char path[4096]; + int i, fd, rc; + + if (argc != 3) { + fprintf(stderr, "usage: %s \n", argv[0]); + return 2; + } + dir = argv[1]; + count = atoi(argv[2]); + for (i = 0; i < count * (WRITERS + 1); ++i) { + name(path, sizeof(path), i); + fd = open(path, O_CREAT | O_WRONLY, 0600); + if (fd == -1) { + perror(path); + return 2; + } + close(fd); + } + rc = pipe_chain(); + if (rc) { + fprintf(stderr, "pipe hand-off failed\n"); + return rc; + } + rc = exit_handoff(); + if (rc) { + fprintf(stderr, "exit hand-off failed\n"); + return rc; + } + return 0; +} diff --git a/test/test-async-order.sh b/test/test-async-order.sh new file mode 100755 index 0000000..5dee635 --- /dev/null +++ b/test/test-async-order.sh @@ -0,0 +1,23 @@ +#!/bin/bash +# +# SPDX-License-Identifier: LGPL-2.1-only +# +# Ownership and mode changes are sent to the server without waiting for a +# reply. Another process that learns of a change, through a pipe or by +# reaping the process that made it, must see the new values. Run it +# both with and without PSEUDO_SYNC, which makes every request wait. +# +# The temporary tree has to live below $PWD so that pseudo tracks it. + +tmpdir="$(mktemp -d -p "$PWD" async_order_XXXXXX)" +trap "rm -rf '$tmpdir'" EXIT TERM INT + +rc=0 +for sync in "" 1; do + rm -f "$tmpdir"/f* + if ! PSEUDO_SYNC=$sync ./test/test-async-order "$tmpdir" 500; then + echo "FAILED: ordering broken (PSEUDO_SYNC=${sync:-unset})" + rc=1 + fi +done +exit $rc