From 0dcb19280adfec5602188a7e1268accf0dffc27f Mon Sep 17 00:00:00 2001 From: Bryan Green Date: Sun, 1 Nov 2026 10:56:33 -0600 Subject: [PATCH v3 1/2] Fix socket handle inheritance on Windows. On Windows, socket handles are inheritable by default, and the server starts its child processes with handle inheritance enabled. Programs run by a backend, via COPY TO PROGRAM, archive_command and so on, therefore inherit the postmaster's listen sockets and the backend's client socket. If the server exits while one of them is still running, the client's connection stays open and the port can't be bound again until the program exits. Fix by adding SOCK_CLOEXEC for backend code on Windows, which pgwin32_socket() maps to WSA_FLAG_NO_HANDLE_INHERIT, and using it for the listen sockets. Also add socket_set_cloexec(), and use it in pq_init() on all platforms in place of fcntl(). Windows needs that too, because the socket handle read_inheritable_socket() creates in the backend is inheritable. Suggested-by: Thomas Munro Discussion: https://postgr.es/m/1387e1ea-ee05-4f5b-a1c4-6c91fddb6d07@gmail.com --- src/backend/libpq/pqcomm.c | 22 ++-- src/backend/port/win32/socket.c | 12 +- src/include/port.h | 3 +- src/include/port/win32_port.h | 6 + src/port/noblock.c | 21 +++- src/test/postmaster/meson.build | 1 + .../postmaster/t/005_handle_inheritance.pl | 105 ++++++++++++++++++ 7 files changed, 157 insertions(+), 13 deletions(-) create mode 100644 src/test/postmaster/t/005_handle_inheritance.pl diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c index 3704d12100..4d2ccb56f7 100644 --- a/src/backend/libpq/pqcomm.c +++ b/src/backend/libpq/pqcomm.c @@ -295,12 +295,9 @@ pq_init(ClientSocket *client_sock) (errmsg("could not set socket to nonblocking mode: %m"))); #endif -#ifndef WIN32 - /* Don't give the socket to any subprograms we execute. */ - if (fcntl(port->sock, F_SETFD, FD_CLOEXEC) < 0) - elog(FATAL, "fcntl(F_SETFD) failed on socket: %m"); -#endif + if (!socket_set_cloexec(port->sock)) + elog(FATAL, "could not set socket to close-on-exec mode: %m"); FeBeWaitSet = CreateWaitEventSet(NULL, FeBeWaitSetNEvents); socket_pos = AddWaitEventToSet(FeBeWaitSet, WL_SOCKET_WRITEABLE, @@ -536,7 +533,13 @@ ListenServerPort(int family, const char *hostName, unsigned short portNumber, addrDesc = addrBuf; } - if ((fd = socket(addr->ai_family, SOCK_STREAM, 0)) == PGINVALID_SOCKET) + /* Don't give the listen socket to any subprograms we execute. */ +#ifdef SOCK_CLOEXEC + fd = socket(addr->ai_family, SOCK_STREAM | SOCK_CLOEXEC, 0); +#else + fd = socket(addr->ai_family, SOCK_STREAM, 0); +#endif + if (fd == PGINVALID_SOCKET) { ereport(LOG, (errcode_for_socket_access(), @@ -545,11 +548,12 @@ ListenServerPort(int family, const char *hostName, unsigned short portNumber, familyDesc, addrDesc))); continue; } +#ifndef SOCK_CLOEXEC + if (!socket_set_cloexec(fd)) + elog(FATAL, "could not set socket to close-on-exec mode: %m"); +#endif #ifndef WIN32 - /* Don't give the listen socket to any subprograms we execute. */ - if (fcntl(fd, F_SETFD, FD_CLOEXEC) < 0) - elog(FATAL, "fcntl(F_SETFD) failed on socket: %m"); /* * Without the SO_REUSEADDR flag, a new postmaster can't be started diff --git a/src/backend/port/win32/socket.c b/src/backend/port/win32/socket.c index 092d41e3f6..7b1cc4a442 100644 --- a/src/backend/port/win32/socket.c +++ b/src/backend/port/win32/socket.c @@ -285,15 +285,23 @@ pgwin32_waitforsinglesocket(SOCKET s, int what, int timeout) } /* - * Create a socket, setting it to overlapped and non-blocking + * Create a socket, setting it to overlapped and non-blocking, and to + * non-inheritable if type includes SOCK_CLOEXEC */ SOCKET pgwin32_socket(int af, int type, int protocol) { SOCKET s; unsigned long on = 1; + DWORD flags = WSA_FLAG_OVERLAPPED; - s = WSASocket(af, type, protocol, NULL, 0, WSA_FLAG_OVERLAPPED); + if (type & SOCK_CLOEXEC) + { + flags |= WSA_FLAG_NO_HANDLE_INHERIT; + type &= ~SOCK_CLOEXEC; + } + + s = WSASocket(af, type, protocol, NULL, 0, flags); if (s == INVALID_SOCKET) { TranslateSocketError(); diff --git a/src/include/port.h b/src/include/port.h index 172acf7d02..0bab9e28ae 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -40,9 +40,10 @@ typedef SOCKET pgsocket; typedef unsigned int socklen_t; #endif -/* non-blocking */ +/* non-blocking, close-on-exec */ extern bool pg_set_noblock(pgsocket sock); extern bool pg_set_block(pgsocket sock); +extern bool socket_set_cloexec(pgsocket sock); /* Portable path handling for Unix/Win32 (in path.c) */ diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h index aff6eaa28f..ecae0cca2e 100644 --- a/src/include/port/win32_port.h +++ b/src/include/port/win32_port.h @@ -518,6 +518,12 @@ extern int pgkill(int pid, int sig); #define recv(s, buf, len, flags) pgwin32_recv(s, buf, len, flags) #define send(s, buf, len, flags) pgwin32_send(s, buf, len, flags) +/* + * pgwin32_socket() maps this to WSA_FLAG_NO_HANDLE_INHERIT. Frontend code + * calls Winsock directly, which would reject it. + */ +#define SOCK_CLOEXEC 0x80000 + extern SOCKET pgwin32_socket(int af, int type, int protocol); extern int pgwin32_bind(SOCKET s, struct sockaddr *addr, int addrlen); extern int pgwin32_listen(SOCKET s, int backlog); diff --git a/src/port/noblock.c b/src/port/noblock.c index c4a6080990..4e4f0cd0be 100644 --- a/src/port/noblock.c +++ b/src/port/noblock.c @@ -1,7 +1,7 @@ /*------------------------------------------------------------------------- * * noblock.c - * set a file descriptor as blocking or non-blocking + * set a file descriptor as blocking or non-blocking, or close-on-exec * * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California @@ -64,3 +64,22 @@ pg_set_block(pgsocket sock) return (ioctlsocket(sock, FIONBIO, &ioctlsocket_ret) == 0); #endif } + +/* + * Put socket into close-on-exec mode. + * Returns true on success, false on failure. + */ +bool +socket_set_cloexec(pgsocket sock) +{ +#if !defined(WIN32) + return (fcntl(sock, F_SETFD, FD_CLOEXEC) != -1); +#else + if (!SetHandleInformation((HANDLE) sock, HANDLE_FLAG_INHERIT, 0)) + { + _dosmaperr(GetLastError()); + return false; + } + return true; +#endif +} diff --git a/src/test/postmaster/meson.build b/src/test/postmaster/meson.build index fa30883b60..af05a9f38f 100644 --- a/src/test/postmaster/meson.build +++ b/src/test/postmaster/meson.build @@ -10,6 +10,7 @@ tests += { 't/002_connection_limits.pl', 't/003_start_stop.pl', 't/004_negotiate.pl', + 't/005_handle_inheritance.pl', ], }, } diff --git a/src/test/postmaster/t/005_handle_inheritance.pl b/src/test/postmaster/t/005_handle_inheritance.pl new file mode 100644 index 0000000000..76a0f86c35 --- /dev/null +++ b/src/test/postmaster/t/005_handle_inheritance.pl @@ -0,0 +1,105 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# On Windows, check that a program started by COPY TO PROGRAM doesn't +# inherit handles that keep the server's resources alive after the server +# is gone. + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; +use Time::HiRes qw(usleep); + +if (!$windows_os) +{ + plan skip_all => 'handle inheritance is specific to Windows'; +} + +my $tempdir = PostgreSQL::Test::Utils::tempdir; +my $ready = "$tempdir/ready"; +my $release = "$tempdir/release"; + +# The program records its PID, then waits until we release it. +my $script = "$tempdir/linger.pl"; +append_to_file( + $script, q{ +my ($ready, $release) = @ARGV; +open(my $fh, '>', "$ready.tmp") or die; +print $fh $$; +close($fh); +rename("$ready.tmp", $ready) or die; +for (1 .. 3000) +{ + last if -e $release; + select(undef, undef, undef, 0.1); +} +}); + +# Wait for $cond to become true, or give up after the default timeout. +sub wait_until +{ + my ($cond) = @_; + + for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++) + { + return 1 if $cond->(); + usleep(100_000); + } + return 0; +} + +# If we die early, still let the program go, or it holds up the test run. +END +{ + eval { append_to_file($release, '') } if defined $release; +} + +my $node = PostgreSQL::Test::Cluster->new('main'); +$node->init; +# Listen on TCP even when the tests use Unix-domain sockets, so that there's a +# listen socket whose port we can check. +$node->append_conf('postgresql.conf', "listen_addresses = '127.0.0.1'"); +$node->start; + +my $psql = $node->background_psql('postgres'); +$psql->query_until( + qr/started/, qq{\\echo started +COPY (SELECT 1) TO PROGRAM '"$^X" "$script" "$ready" "$release"'; +}); + +wait_until(sub { -e $ready }) or die "COPY TO PROGRAM did not start"; +my $pid = slurp_file($ready); + +# This takes a few seconds: the backend is waiting for the program, so it +# doesn't see the shutdown signal, and the postmaster has to kill it. +$node->stop('immediate'); + +SKIP: +{ + skip 'program did not outlive the server', 2 unless kill(0, $pid); + + # The client socket: the client should see its connection drop. The + # timer throws if it expires, so catch that and report a failure. + $psql->{timeout}->start($PostgreSQL::Test::Utils::timeout_default); + my $closed = eval { + pump_until( + $psql->{run}, $psql->{timeout}, \$psql->{stderr}, + qr/server closed the connection unexpectedly|connection to server was lost/ + ); + }; + ok($closed, 'client connection closed while program is still running'); + + # The listen sockets: the port should be free for a new postmaster. + ok(PostgreSQL::Test::Cluster::can_bind('127.0.0.1', $node->port), + 'port is free while program is still running'); +} + +append_to_file($release, ''); +wait_until(sub { !kill(0, $pid) }) or die "program did not exit"; + +# psql has exited by now, having lost its connection, so don't try \q. +$psql->{run}->finish; + +done_testing(); -- 2.54.0.windows.1