From 4bed432347c7e8ff428ee57fd1d5bf4a1017a5a8 Mon Sep 17 00:00:00 2001 From: Bryan Green Date: Sun, 1 Nov 2026 11:30:40 -0600 Subject: [PATCH v3 2/2] Fix shared memory handle inheritance on Windows. The postmaster made its handle to the main shared memory segment inheritable, so that child processes could use the same handle value. Programs run by those children, via COPY TO PROGRAM, archive_command and so on, then inherited it as well. If the server exits while one of them is still running, the segment stays in use, and a new postmaster fails with "pre-existing shared memory block is still in use" until the program exits. Fix by keeping the postmaster's handle non-inheritable, and duplicating it into each child in save_backend_variables(), as is already done for the signal pipe. The child's copies of both are now non-inheritable. Discussion: https://postgr.es/m/1387e1ea-ee05-4f5b-a1c4-6c91fddb6d07@gmail.com --- src/backend/port/win32_shmem.c | 46 ++++++------------- src/backend/postmaster/launch_backend.c | 21 +++++++-- .../postmaster/t/005_handle_inheritance.pl | 8 +++- 3 files changed, 40 insertions(+), 35 deletions(-) diff --git a/src/backend/port/win32_shmem.c b/src/backend/port/win32_shmem.c index 794e4fcb2a..72a61596d6 100644 --- a/src/backend/port/win32_shmem.c +++ b/src/backend/port/win32_shmem.c @@ -209,8 +209,7 @@ PGSharedMemoryCreate(Size size, { void *memAddress; PGShmemHeader *hdr; - HANDLE hmap, - hmap2; + HANDLE hmap; char *szShareMem; int i; DWORD size_high; @@ -341,21 +340,6 @@ retry: free(szShareMem); - /* - * Make the handle inheritable - */ - if (!DuplicateHandle(GetCurrentProcess(), hmap, GetCurrentProcess(), &hmap2, 0, TRUE, DUPLICATE_SAME_ACCESS)) - ereport(FATAL, - (errmsg("could not create shared memory segment: error code %lu", GetLastError()), - errdetail("Failed system call was DuplicateHandle."))); - - /* - * Close the old, non-inheritable handle. If this fails we don't really - * care. - */ - if (!CloseHandle(hmap)) - elog(LOG, "could not close handle to shared memory: error code %lu", GetLastError()); - desiredAccess = FILE_MAP_WRITE | FILE_MAP_READ; #ifdef FILE_MAP_LARGE_PAGES @@ -368,7 +352,7 @@ retry: * Get a pointer to the new shared memory segment. Map the whole segment * at once, and let the system decide on the initial address. */ - memAddress = MapViewOfFileEx(hmap2, desiredAccess, 0, 0, 0, NULL); + memAddress = MapViewOfFileEx(hmap, desiredAccess, 0, 0, 0, NULL); if (!memAddress) ereport(FATAL, (errmsg("could not create shared memory segment: error code %lu", GetLastError()), @@ -395,10 +379,10 @@ retry: /* Save info for possible future use */ UsedShmemSegAddr = memAddress; UsedShmemSegSize = size; - UsedShmemSegID = hmap2; + UsedShmemSegID = hmap; /* Register on-exit routine to delete the new segment */ - on_shmem_exit(pgwin32_SharedMemoryDelete, PointerGetDatum(hmap2)); + on_shmem_exit(pgwin32_SharedMemoryDelete, PointerGetDatum(hmap)); *shim = hdr; @@ -413,12 +397,12 @@ retry: * PGSharedMemoryReAttach * * This is called during startup of a postmaster child process to re-attach to - * an already existing shared memory segment, using the handle inherited from - * the postmaster. + * an already existing shared memory segment, using the handle the postmaster + * duplicated for it. * * ShmemProtectiveRegion, UsedShmemSegID and UsedShmemSegAddr are implicit - * parameters to this routine. The caller must have already restored them to - * the postmaster's values. + * parameters to this routine. The caller must have already restored them + * from the backend parameters. */ void PGSharedMemoryReAttach(void) @@ -465,8 +449,8 @@ PGSharedMemoryReAttach(void) * after this; make sure that it will be a no-op if called. * * ShmemProtectiveRegion, UsedShmemSegID and UsedShmemSegAddr are implicit - * parameters to this routine. The caller must have already restored them to - * the postmaster's values. + * parameters to this routine. The caller must have already restored them + * from the backend parameters. */ void PGSharedMemoryNoReAttach(void) @@ -482,9 +466,9 @@ PGSharedMemoryNoReAttach(void) UsedShmemSegAddr = NULL; /* - * We *must* close the inherited shmem segment handle, else Windows will - * consider the existence of this process to mean it can't release the - * shmem segment yet. We can now use PGSharedMemoryDetach to do that. + * We *must* close our shmem segment handle, else Windows will consider + * the existence of this process to mean it can't release the shmem + * segment yet. We can now use PGSharedMemoryDetach to do that. */ PGSharedMemoryDetach(); } @@ -495,8 +479,8 @@ PGSharedMemoryNoReAttach(void) * Detach from the shared memory segment, if still attached. This is not * intended to be called explicitly by the process that originally created the * segment (it will have an on_shmem_exit callback registered to do that). - * Rather, this is for subprocesses that have inherited an attachment and want - * to get rid of it. + * Rather, this is for subprocesses that have been given an attachment and + * want to get rid of it. * * ShmemProtectiveRegion, UsedShmemSegID and UsedShmemSegAddr are implicit * parameters to this routine. diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index 093ef87912..6221dc1caf 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -726,8 +726,22 @@ save_backend_variables(BackendParameters *param, #ifdef WIN32 param->ShmemProtectiveRegion = ShmemProtectiveRegion; -#endif + + /* Give the child its own handle to the segment, not inheritable */ + param->UsedShmemSegID = INVALID_HANDLE_VALUE; + if (UsedShmemSegID != INVALID_HANDLE_VALUE && + !DuplicateHandle(GetCurrentProcess(), UsedShmemSegID, + childProcess, ¶m->UsedShmemSegID, + 0, FALSE, DUPLICATE_SAME_ACCESS)) + { + ereport(LOG, + (errmsg_internal("could not duplicate shared memory handle for child process: error code %lu", + GetLastError()))); + return false; + } +#else param->UsedShmemSegID = UsedShmemSegID; +#endif param->UsedShmemSegAddr = UsedShmemSegAddr; #ifdef USE_INJECTION_POINTS @@ -782,7 +796,8 @@ save_backend_variables(BackendParameters *param, #ifdef WIN32 /* * Duplicate a handle for usage in a child process, and write the child - * process instance of the handle to the parameter file. + * process instance of the handle to the parameter file. The child's copy + * is not inheritable. */ static bool write_duplicated_handle(HANDLE *dest, HANDLE src, HANDLE childProcess) @@ -794,7 +809,7 @@ write_duplicated_handle(HANDLE *dest, HANDLE src, HANDLE childProcess) childProcess, &hChild, 0, - TRUE, + FALSE, DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS)) { ereport(LOG, diff --git a/src/test/postmaster/t/005_handle_inheritance.pl b/src/test/postmaster/t/005_handle_inheritance.pl index 76a0f86c35..69091759cf 100644 --- a/src/test/postmaster/t/005_handle_inheritance.pl +++ b/src/test/postmaster/t/005_handle_inheritance.pl @@ -78,7 +78,7 @@ $node->stop('immediate'); SKIP: { - skip 'program did not outlive the server', 2 unless kill(0, $pid); + skip 'program did not outlive the server', 3 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. @@ -94,6 +94,12 @@ SKIP: # 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'); + + # The shared memory segment: a new postmaster should start. The program + # does still hold the old log file as its stderr, so give the new + # postmaster another one. + $node->rotate_logfile; + ok($node->start(fail_ok => 1), 'restart while program is still running'); } append_to_file($release, ''); -- 2.54.0.windows.1