From 48b34a47a6d82efb4e193c29eb8558f7e881316a Mon Sep 17 00:00:00 2001 From: Levi Neely <141506390+lneely@users.noreply.github.com> Date: Wed, 4 Mar 2026 19:13:18 +0100 Subject: [PATCH] Fix pcl-zqv.4.3: socket lifecycle and fd management in psock.c (#349) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - connect_res(): add SO_ERROR check after EINPROGRESS + wait_writable; a refused/reset connection makes the socket writable with SO_ERROR set — previously returned the socket as 'connected' when it was not. Added psock_check_so_error() helper using getsockopt(SO_ERROR). - wait_readable(), wait_writable(), wait_ssl_ready(): wrap select() in do{...}while(errno==EINTR) with fd_set reinit on each iteration. Signal delivery during blocking waits previously caused immediate SOCKET_ERROR return, dropping live connections on any signal. Linux updates tv on EINTR to reflect remaining time; reinitializing fd_set before each retry ensures a clean select state. - psync_socket_read_plain(), psync_socket_readall_plain(), psync_socket_readall_plain_thread(): add EINTR to EAGAIN/EWOULDBLOCK retry condition on read(). EINTR is a non-fatal interruption; retrying is correct and consistent with the non-blocking socket model. - psync_socket_read_noblock_plain(): return PSYNC_SOCKET_WOULDBLOCK on EINTR rather than -1 (fatal error); callers handle WOULDBLOCK by scheduling a retry. - psync_socket_writeall_plain(), psync_socket_writeall_plain_thread(): add EINTR retry path on write() without a write-timeout wait, since EINTR does not indicate the socket became unwritable. - psock_connect(): add null check on malloc(sizeof(psock_t)); on OOM, close the connected fd and free the SSL context before returning NULL. Co-authored-by: Levi Neely Co-authored-by: Claude Sonnet 4.6 --- pclsync/psock.c | 70 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 48 insertions(+), 22 deletions(-) diff --git a/pclsync/psock.c b/pclsync/psock.c index 1272ad2..4ea69a6 100644 --- a/pclsync/psock.c +++ b/pclsync/psock.c @@ -62,10 +62,12 @@ static int wait_readable(int sock, long sec, long usec) { int res; tv.tv_sec = sec; tv.tv_usec = usec; - FD_ZERO(&rfds); - FD_SET(sock, &rfds); clock_gettime(CLOCK_REALTIME, &start); - res = select(sock + 1, &rfds, NULL, NULL, &tv); + do { + FD_ZERO(&rfds); + FD_SET(sock, &rfds); + res = select(sock + 1, &rfds, NULL, NULL, &tv); + } while (res == -1 && errno == EINTR); if (res == 1) { psock_debug_log_wait_latency(&start); @@ -87,9 +89,11 @@ static int wait_writable(int sock, long sec, long usec) { int res; tv.tv_sec = sec; tv.tv_usec = usec; - FD_ZERO(&wfds); - FD_SET(sock, &wfds); - res = select(sock + 1, NULL, &wfds, NULL, &tv); + do { + FD_ZERO(&wfds); + FD_SET(sock, &wfds); + res = select(sock + 1, NULL, &wfds, NULL, &tv); + } while (res == -1 && errno == EINTR); if (res == 1) return 0; if (res == 0) @@ -98,18 +102,14 @@ static int wait_writable(int sock, long sec, long usec) { } static int wait_ssl_ready(int sock) { - fd_set fds, *rfds, *wfds; + fd_set fds; struct timeval tv; - int res; - FD_ZERO(&fds); - FD_SET(sock, &fds); + int res, want_read; if (psync_ssl_errno == PSYNC_SSL_ERR_WANT_READ) { - rfds = &fds; - wfds = NULL; + want_read = 1; tv.tv_sec = PSYNC_SOCK_READ_TIMEOUT; } else if (psync_ssl_errno == PSYNC_SSL_ERR_WANT_WRITE) { - rfds = NULL; - wfds = &fds; + want_read = 0; tv.tv_sec = PSYNC_SOCK_WRITE_TIMEOUT; } else { pdbg_logf(D_BUG, "this functions should only be called when SSL returns " @@ -118,7 +118,14 @@ static int wait_ssl_ready(int sock) { return SOCKET_ERROR; } tv.tv_usec = 0; - res = select(sock + 1, rfds, wfds, NULL, &tv); + do { + FD_ZERO(&fds); + FD_SET(sock, &fds); + res = select(sock + 1, + want_read ? &fds : NULL, + want_read ? NULL : &fds, + NULL, &tv); + } while (res == -1 && errno == EINTR); if (res == 1) return 0; if (res == 0) { @@ -229,6 +236,14 @@ static void addr_save(const char *host, const char *port, psql_commit(); } +static int psock_check_so_error(int sock) { + int err; + socklen_t len = sizeof(err); + return (getsockopt(sock, SOL_SOCKET, SO_ERROR, &err, &len) == 0 && err == 0) + ? 0 + : -1; +} + static int connect_res(struct addrinfo *res) { int sock; #if defined(SOCK_NONBLOCK) @@ -251,7 +266,8 @@ static int connect_res(struct addrinfo *res) { #endif if ((connect(sock, res->ai_addr, res->ai_addrlen) != SOCKET_ERROR) || (errno == EINPROGRESS && - !wait_writable(sock, PSYNC_SOCK_CONNECT_TIMEOUT, 0))) + !wait_writable(sock, PSYNC_SOCK_CONNECT_TIMEOUT, 0) && + !psock_check_so_error(sock))) return sock; close(sock); } @@ -474,6 +490,12 @@ psock_t *psock_connect(const char *host, int unsigned port, int ssl) { sslc = NULL; } ret = malloc(sizeof(psock_t)); + if (!ret) { + if (sslc) + pssl_free(sslc); + close(sock); + return NULL; + } ret->ssl = sslc; ret->buffer = NULL; ret->sock = sock; @@ -635,7 +657,7 @@ static int psync_socket_read_plain(psock_t *sock, void *buff, int num) { psock_try_write_buffer(sock); r = read(sock->sock, buff, num); if (r == SOCKET_ERROR) { - if (pdbg_likely(errno == EWOULDBLOCK || errno == EAGAIN)) + if (pdbg_likely(errno == EWOULDBLOCK || errno == EAGAIN || errno == EINTR)) continue; else return -1; @@ -672,7 +694,7 @@ static int psync_socket_read_noblock_plain(psock_t *sock, void *buff, int num) { r = read(sock->sock, buff, num); if (r == SOCKET_ERROR) { sock->pending = 0; - if (pdbg_likely(errno == EWOULDBLOCK || errno == EAGAIN)) + if (pdbg_likely(errno == EWOULDBLOCK || errno == EAGAIN || errno == EINTR)) return PSYNC_SOCKET_WOULDBLOCK; else return -1; @@ -863,7 +885,7 @@ static int psync_socket_readall_plain(psock_t *sock, void *buff, int num) { psock_try_write_buffer(sock); r = read(sock->sock, (char *)buff + br, num - br); if (r == SOCKET_ERROR) { - if (pdbg_likely(errno == EWOULDBLOCK || errno == EAGAIN)) + if (pdbg_likely(errno == EWOULDBLOCK || errno == EAGAIN || errno == EINTR)) continue; else return -1; @@ -918,7 +940,9 @@ static int psync_socket_writeall_plain(int sock, const void *buff, int num) { return -1; else continue; - } else + } else if (errno == EINTR) + continue; + else return -1; } br += r; @@ -986,7 +1010,7 @@ static int psync_socket_readall_plain_thread(psock_t *sock, void *buff, r = read(sock->sock, (char *)buff + br, num - br); pthread_mutex_unlock(&mutex); if (r == SOCKET_ERROR) { - if (pdbg_likely(errno == EWOULDBLOCK || errno == EAGAIN)) + if (pdbg_likely(errno == EWOULDBLOCK || errno == EAGAIN || errno == EINTR)) continue; else return -1; @@ -1052,7 +1076,9 @@ static int psync_socket_writeall_plain_thread(psock_t *sock, const void *buff, return -1; else continue; - } else + } else if (errno == EINTR) + continue; + else return -1; } br += r;