Обсуждение: [PATCH] Fix orphaned backend processes on Windows using Job Objects
Greetings, When the postmaster exits unexpectedly on Windows (crash, kill, debugger abort), backend processes continue running. Windows lacks any equivalent to Unix's getppid() orphan detection. These orphaned backends hold locks and shared memory, preventing clean restart. This leads to a delay in restarts and manual killing of orphans. The problem is easy to reproduce. Start postgres, open a transaction with LOCK TABLE, then kill the postmaster with taskkill /F. The backend continues running and restart fails. Manual cleanup is required. Current approaches (inherited event handles, shared memory flags) depend on the postmaster running code during exit. A segfault or kill bypasses all of that. My proposed solution is to use Windows Job Objects with KILL_ON_JOB_CLOSE. We just need to call CreateJobObject() in PostmasterMain(), configure with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, and assign the postmaster. Children inherit membership automatically. When the job handle closes on postmaster exit, the kernel terminates all children atomically. This is kernel-enforced with no polling and no race conditions. Job creation can fail if postgres runs under an existing job (service managers, debuggers). Windows 7 disallows nested jobs. We detect this with IsProcessInJob(), and if AssignProcessToJobObject() returns ERROR_ACCESS_DENIED, we log and continue without orphan protection. KILL_ON_JOB_CLOSE doesn't interfere with clean shutdown. Normal shutdown signals backends via SetEvent, they exit, postmaster exits, job closes. Nothing left to kill. The flag only fires during crashes when backends are still running - exactly when forced termination is correct. The code is ~200 lines in pg_job_object.c, less than win32/signal.c (~500 lines). It fails gracefully and works regardless of how postgres is started, unlike service manager approaches. This avoids polling unreliability. The patch has been tested on Windows 10/11 with both MSVC and MinGW builds. Nested jobs fail gracefully as expected. Clean shutdown is unaffected. Crash tests with taskkill /F, debugger abort, and access violations all correctly terminate children immediately with zero orphans. This patch does not include automated tests because the core functionality (orphan prevention on crash) requires simulating process termination, which is difficult to test reliably in CI. Patch attached. Can add documentation if this approach is approved. Thoughts? Bryan Green
Вложения
Hi, On 2025-11-03 09:12:03 -0600, Bryan Green wrote: > We just need to call CreateJobObject() in PostmasterMain(), configure > with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, and assign the postmaster. > Children inherit membership automatically. When the job handle closes on > postmaster exit, the kernel terminates all children atomically. This is > kernel-enforced with no polling and no race conditions. What happens if a postmaster child exits irregularly? Is postmaster terminated as well? > The patch has been tested on Windows 10/11 with both MSVC and MinGW > builds. Nested jobs fail gracefully as expected. Clean shutdown is > unaffected. Crash tests with taskkill /F, debugger abort, and access > violations all correctly terminate children immediately with zero orphans. > > This patch does not include automated tests because the core > functionality (orphan prevention on crash) requires simulating process > termination, which is difficult to test reliably in CI. Why is it difficult to test in CI? We do some related tests in 013_crash_restart.pl, it doesn't seem like it ought to be hard to also add tests for postmaster? Greetings, Andres Freund
On 11/3/2025 9:19 AM, Andres Freund wrote: > Hi, > > On 2025-11-03 09:12:03 -0600, Bryan Green wrote: >> We just need to call CreateJobObject() in PostmasterMain(), configure >> with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, and assign the postmaster. >> Children inherit membership automatically. When the job handle closes on >> postmaster exit, the kernel terminates all children atomically. This is >> kernel-enforced with no polling and no race conditions. > > What happens if a postmaster child exits irregularly? Is postmaster terminated > as well? > No, Job Objects are unidirectional. KILL_ON_JOB_CLOSE only acts when the postmaster (which holds the job handle) exits. Backend crashes are handled through PostgreSQL's existing crash recovery mechanism - the postmaster detects the crash via WaitForMultipleObjects() and initiates recovery as normal. The Job Object only takes action when the job handle closes, which happens when the postmaster exits. It's analogous to a Unix process group - sending SIGTERM to the group leader kills the group, but children dying doesn't affect the parent. >> The patch has been tested on Windows 10/11 with both MSVC and MinGW >> builds. Nested jobs fail gracefully as expected. Clean shutdown is >> unaffected. Crash tests with taskkill /F, debugger abort, and access >> violations all correctly terminate children immediately with zero orphans. >> >> This patch does not include automated tests because the core >> functionality (orphan prevention on crash) requires simulating process >> termination, which is difficult to test reliably in CI. > > Why is it difficult to test in CI? We do some related tests in > 013_crash_restart.pl, it doesn't seem like it ought to be hard to also add > tests for postmaster? > Fair point. I was hesitant because testing the actual orphan prevention requires killing the postmaster while backends are active, which seemed fragile. But you're right that we already test similar scenarios. I can add a test to 013_crash_restart.pl (or a new Windows-specific test file) that: 1. Starts server with active backend 2. Kills postmaster ungracefully (taskkill /F) 3. Verifies backend process terminates automatically 4. Confirms clean restart Would that be sufficient, or do you have other test scenarios in mind? > Greetings, > > Andres Freund
On 2025-11-03 09:25:11 -0600, Bryan Green wrote: > On 11/3/2025 9:19 AM, Andres Freund wrote: > > Hi, > > > > On 2025-11-03 09:12:03 -0600, Bryan Green wrote: > >> We just need to call CreateJobObject() in PostmasterMain(), configure > >> with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, and assign the postmaster. > >> Children inherit membership automatically. When the job handle closes on > >> postmaster exit, the kernel terminates all children atomically. This is > >> kernel-enforced with no polling and no race conditions. > > > > What happens if a postmaster child exits irregularly? Is postmaster terminated > > as well? > > > > No, Job Objects are unidirectional. Great. > >> The patch has been tested on Windows 10/11 with both MSVC and MinGW > >> builds. Nested jobs fail gracefully as expected. Clean shutdown is > >> unaffected. Crash tests with taskkill /F, debugger abort, and access > >> violations all correctly terminate children immediately with zero orphans. > >> > >> This patch does not include automated tests because the core > >> functionality (orphan prevention on crash) requires simulating process > >> termination, which is difficult to test reliably in CI. > > > > Why is it difficult to test in CI? We do some related tests in > > 013_crash_restart.pl, it doesn't seem like it ought to be hard to also add > > tests for postmaster? > > > > Fair point. I was hesitant because testing the actual orphan prevention > requires killing the postmaster while backends are active, which seemed > fragile. But you're right that we already test similar scenarios. > > I can add a test to 013_crash_restart.pl (or a new Windows-specific test > file) that: > 1. Starts server with active backend > 2. Kills postmaster ungracefully (taskkill /F) > 3. Verifies backend process terminates automatically > 4. Confirms clean restart > > Would that be sufficient, or do you have other test scenarios in mind? That's pretty much what I had in mind. Greetings, Andres Freund
On 11/3/2025 9:29 AM, Andres Freund wrote: > On 2025-11-03 09:25:11 -0600, Bryan Green wrote: >> On 11/3/2025 9:19 AM, Andres Freund wrote: >>> Hi, >>> >>> On 2025-11-03 09:12:03 -0600, Bryan Green wrote: >>>> We just need to call CreateJobObject() in PostmasterMain(), configure >>>> with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, and assign the postmaster. >>>> Children inherit membership automatically. When the job handle closes on >>>> postmaster exit, the kernel terminates all children atomically. This is >>>> kernel-enforced with no polling and no race conditions. >>> >>> What happens if a postmaster child exits irregularly? Is postmaster terminated >>> as well? >>> >> >> No, Job Objects are unidirectional. > > Great. > > >>>> The patch has been tested on Windows 10/11 with both MSVC and MinGW >>>> builds. Nested jobs fail gracefully as expected. Clean shutdown is >>>> unaffected. Crash tests with taskkill /F, debugger abort, and access >>>> violations all correctly terminate children immediately with zero orphans. >>>> >>>> This patch does not include automated tests because the core >>>> functionality (orphan prevention on crash) requires simulating process >>>> termination, which is difficult to test reliably in CI. >>> >>> Why is it difficult to test in CI? We do some related tests in >>> 013_crash_restart.pl, it doesn't seem like it ought to be hard to also add >>> tests for postmaster? >>> >> >> Fair point. I was hesitant because testing the actual orphan prevention >> requires killing the postmaster while backends are active, which seemed >> fragile. But you're right that we already test similar scenarios. >> >> I can add a test to 013_crash_restart.pl (or a new Windows-specific test >> file) that: >> 1. Starts server with active backend >> 2. Kills postmaster ungracefully (taskkill /F) >> 3. Verifies backend process terminates automatically >> 4. Confirms clean restart >> >> Would that be sufficient, or do you have other test scenarios in mind? > > That's pretty much what I had in mind. > > Greetings, > > Andres Freund I've implemented the test in 013_crash_restart.pl. The test passes on Windows 10/11 with both MSVC and MinGW builds. Backends are typically terminated within 100-200ms after postmaster kill, confirming the Job Object KILL_ON_JOB_CLOSE mechanism works as intended. Updated patch (v2) attached. -- Bryan
Вложения
On Tue, Nov 4, 2025 at 4:12 AM Bryan Green <dbryan.green@gmail.com> wrote: > Current approaches (inherited event handles, shared memory flags) depend > on the postmaster running code during exit. A segfault or kill bypasses > all of that. Huh. I thought PostmasterHandle should be signalled by the kernel, not by any code run by the postmaster itself, when taskkill /f calls something like TerminateProcess(). Is that not the case? Are you sure we haven't broken something on our side? > My proposed solution is to use Windows Job Objects with KILL_ON_JOB_CLOSE. I'm not a Windows guy but I had been researching this independently, and it does seem to be the standard approach, so +1 generally even though I'm still very curious to know *why* the existing coding doesn't work in such a simple case. By coincidence, I'm getting close to posting a stack of patches for better postmaster death handling on Unix too, along with better subprocess and interrupt multiplexing and cleanup. That does more stuff, with connections to multithreading, interrupts, critical sections, a bunch of existing bugs we have with subprocess management on Unix. I will gladly delete my own attempt at Windows job objects from that effort and rebase on top of your patch, and see what review feedback ideas come up in that process. In nearby threads that triggered my work on that, I was a bit worried about the change in behaviour on PM death in the syncrep and syslogger, but I'm beginning to suspect that the vast majority of Linux/systemd deployments probably just nuke the whole cgroup from orbit in this case, so it seems like exceptional behaviour is really just a recipe for fragility on rarer systems, not to mention that it probably has to be like that in a potential multithreaded mode anyway. We should probably just rip all such specialness out, which I'll show in my patch set with some explanations soon. On the other hand, I was thinking of this as a v19 feature, where one can contemplate such changes, but you said: > Job creation can fail if postgres runs under an existing job (service > managers, debuggers). Windows 7 disallows nested jobs. We detect this > with IsProcessInJob(), and if AssignProcessToJobObject() returns > ERROR_ACCESS_DENIED, we log and continue without orphan protection. We currently require Windows 10 (itself recently EOL'd), but PostgreSQL 13-15 sort of claim to work on Windows all the way back to 7, so I'm guessing you're imagining back-patching this? > This patch does not include automated tests because the core > functionality (orphan prevention on crash) requires simulating process > termination, which is difficult to test reliably in CI. Ah yes I ran into problems with that part too...
On 11/6/2025 6:46 AM, Thomas Munro wrote: > On Tue, Nov 4, 2025 at 4:12 AM Bryan Green <dbryan.green@gmail.com> wrote: >> Current approaches (inherited event handles, shared memory flags) depend >> on the postmaster running code during exit. A segfault or kill bypasses >> all of that. > > Huh. I thought PostmasterHandle should be signalled by the kernel, > not by any code run by the postmaster itself, when taskkill /f > calls something like TerminateProcess(). Is that not the case? Are > you sure we haven't broken something on our side? > Yes, that is true. I may have been a bit sloppy with my wording here. This patch is focused solely on quickly terminating the backend processes when postmaster has crashed. Any children created in a suspended state will not handle the signal. Additionally, we have handles being inherited by the children that keep them from terminating. See Fix Socket handle Inheritance on Windows (https://commitfest.postgresql.org/patch/6207/) and O_CLOEXEC not honored on Windows - handle inheritance chain (https://commitfest.postgresql.org/patch/6197/). When the children are in a suspended state or have handles inherited from the parent process then we have to wait-- which we shouldn't do when postmaster crashes. The reason to still do this patch and clean up the handle inheritance mess is that there are states (suspended state, infinite loop, spinlock hold, whatever) that a process can be in that keeps it from processing the event. We don't need to wait on the children to voluntarily exit when postmaster crashes. >> My proposed solution is to use Windows Job Objects with KILL_ON_JOB_CLOSE. > > I'm not a Windows guy but I had been researching this independently, > and it does seem to be the standard approach, so +1 generally even > though I'm still very curious to know *why* the existing coding > doesn't work in such a simple case. > > By coincidence, I'm getting close to posting a stack of patches for > better postmaster death handling on Unix too, along with better > subprocess and interrupt multiplexing and cleanup. That does more > stuff, with connections to multithreading, interrupts, critical > sections, a bunch of existing bugs we have with subprocess management > on Unix. I will gladly delete my own attempt at Windows job objects > from that effort and rebase on top of your patch, and see what review > feedback ideas come up in that process. > That sounds excellent. I'd be happy to coordinate - please feel free to build on this work. The Windows Job Object approach is fairly self-contained, so it should integrate cleanly with your broader subprocess management improvements. > In nearby threads that triggered my work on that, I was a bit worried > about the change in behaviour on PM death in the syncrep and > syslogger, but I'm beginning to suspect that the vast majority of > Linux/systemd deployments probably just nuke the whole cgroup from > orbit in this case, so it seems like exceptional behaviour is really > just a recipe for fragility on rarer systems, not to mention that it > probably has to be like that in a potential multithreaded mode anyway. > We should probably just rip all such specialness out, which I'll show > in my patch set with some explanations soon. > > On the other hand, I was thinking of this as a v19 feature, where one > can contemplate such changes, but you said: > >> Job creation can fail if postgres runs under an existing job (service >> managers, debuggers). Windows 7 disallows nested jobs. We detect this >> with IsProcessInJob(), and if AssignProcessToJobObject() returns >> ERROR_ACCESS_DENIED, we log and continue without orphan protection. > > We currently require Windows 10 (itself recently EOL'd), but > PostgreSQL 13-15 sort of claim to work on Windows all the way back to > 7, so I'm guessing you're imagining back-patching this? > No, I'm targeting v19 as a new feature. The Windows 7 comment was just being defensive about the failure mode - since we allow job creation to fail gracefully, there's no harm in having the code check for nested job scenarios even though we don't officially support Win7 anymore. The code degrades gracefully to "current behavior" if job creation fails for any reason. >> This patch does not include automated tests because the core >> functionality (orphan prevention on crash) requires simulating process >> termination, which is difficult to test reliably in CI. > > Ah yes I ran into problems with that part too... Thanks for the review, I look forward to working on this problem area with you. It's good to have more than one set of eyes looking at a problem. -- Bryan Green EDB: https://www.enterprisedb.com
On Fri, Nov 7, 2025 at 3:13 AM Bryan Green <dbryan.green@gmail.com> wrote: > The reason to still do this patch and clean up the handle inheritance > mess is that there are states (suspended state, infinite loop, spinlock > hold, whatever) that a process can be in that keeps it from processing > the event. We don't need to wait on the children to voluntarily exit > when postmaster crashes. Agreed on all points. We'd recently come to the same conclusion on this thread: https://www.postgresql.org/message-id/flat/B3C69B86-7F82-4111-B97F-0005497BB745%40yandex-team.ru I think there might arguably be a sort of weak forward progress guarantee in the existing design and it's been a while since we've had problem reports AFAIR*: locks were releases (which turns out to be fundamentally unsafe at least while in a critical section as analysed in that thread, but it does allow progress in blocked backends, so that they can learn of the postmaster's demise), and no one should enter WaitEventSet() while holding a spinlock, and infinite loops are against the law, and it's previously been considered acceptable-ish that a backend might continue to run a long query until completion before exiting (without supporting auxiliary or worker backends, which sounds potentially suspect, but at least you can't wait for another backend without learning of the PostgreSQL's demise assuming the only possible waits are LWLocks or latches). But clearly it's not good enough. The fact that Windows backends are born in suspended state until the postmaster resumes them is indeed a new and significant hole in that theory. Preemptive termination is the only thing that makes sense. *We used to have places that waited but forgot to handle PM exit, and I don't recall "manual orphan cleanup needed" reports since we enforced a central handler. But see also my earlier note about systemd potentially hiding problems these days, if using "mixed" mode to SIGKILL the whole cgroup.
On 11/6/2025 8:39 PM, Thomas Munro wrote: > On Fri, Nov 7, 2025 at 3:13 AM Bryan Green <dbryan.green@gmail.com> wrote: >> The reason to still do this patch and clean up the handle inheritance >> mess is that there are states (suspended state, infinite loop, spinlock >> hold, whatever) that a process can be in that keeps it from processing >> the event. We don't need to wait on the children to voluntarily exit >> when postmaster crashes. > > Agreed on all points. We'd recently come to the same conclusion on this thread: > > https://www.postgresql.org/message-id/flat/B3C69B86-7F82-4111-B97F-0005497BB745%40yandex-team.ru > Thanks for the link - I'll review that thread. It's reassuring to see independent analysis reaching the same conclusions. > I think there might arguably be a sort of weak forward progress > guarantee in the existing design and it's been a while since we've had > problem reports AFAIR*: locks were releases (which turns out to be > fundamentally unsafe at least while in a critical section as analysed > in that thread, but it does allow progress in blocked backends, so > that they can learn of the postmaster's demise), and no one should > enter WaitEventSet() while holding a spinlock, and infinite loops are > against the law, and it's previously been considered acceptable-ish > that a backend might continue to run a long query until completion > before exiting (without supporting auxiliary or worker backends, which > sounds potentially suspect, but at least you can't wait for another > backend without learning of the PostgreSQL's demise assuming the only > possible waits are LWLocks or latches). But clearly it's not good > enough. > > The fact that Windows backends are born in suspended state until the > postmaster resumes them is indeed a new and significant hole in that > theory. Preemptive termination is the only thing that makes sense. > Exactly. The suspended state issue-- even with perfect handle inheritance hygiene, a backend that hasn't been resumed yet cannot receive or process any event notification. The postmaster could crash after CreateProcess() but before ResumeThread(), leaving a zombie process that will never wake up. The handle inheritance problems I've been working on (socket handles, O_CLOEXEC) are somewhat orthogonal - they cause *delayed* termination rather than indefinite orphaning. But Job Objects solves both classes of problems: it handles the suspended state case immediately, and provides guaranteed cleanup regardless of handle state. > *We used to have places that waited but forgot to handle PM exit, and > I don't recall "manual orphan cleanup needed" reports since we > enforced a central handler. But see also my earlier note about > systemd potentially hiding problems these days, if using "mixed" mode > to SIGKILL the whole cgroup. That's an interesting observation about systemd potentially masking fragility. If Linux deployments are really just nuking the cgroup on postmaster death, then the "voluntary exit" approach isn't actually being tested in production at scale. I'm looking forward to your broader subprocess management patchset. -- Bryan Green EDB: https://www.enterprisedb.com