Re: Can we get rid of TerminateThread() in pg_dump?

On Sat, Jul 4, 2026 at 1:56 AM Bryan Green <dbryan(dot)green(at)gmail(dot)com> wrote:
> It works. It compiles clean, and on a ^C during a parallel dump it
> suppresses the worker cancel messages the same way the TerminateThread
> call did-- I checked against unmodified master with the TerminateThread
> call still in place, and the output is identical-- "terminated by user"
> and nothing else.

Thank you for confirming! So now I'm wondering, do we want to let
sleeping dogs lie, or do we want to back-patch this? If we're happy
to leave the back-branches, which have never received a user
complaint, then I'll simply adopt this approach in my
pg_dump-with-threads-on-Unix-too patch set (which I'll post shortly),
ie the same dup2() code will be used on all systems. But if we think
this problem is worth fixing in back-branches, we could back-patch
this stand-alone patch first. Thoughts?

> Moving write_stderr off fileno(stderr) to STDERR_FILENO is worth having
> on its own: that shim's comment notes _fileno returns -1 when the stream
> is closed, which is a state the shutdown path can be in.

Cool. Yeah, I failed to mention why I'd done that in my previous
email, and that was it.

> Opening NUL in the handler worked here; I didn't need to open it in advance.

I was worrying that open() might fail with EMFILE. I don't think it's
really going to happen though, given the bounded numbers of files that
pg_dump/pg_restore should be working with, so maybe it's OK as it is.

> Since you mention moving parallel.c to threads on non-Windows, I've been
> replacing the socketpair/select() worker protocol on Windows with an
> in-process queue (a mutex and two condition variables)[1], heading the
> same direction to unify across platforms. There a worker signals its own
> death through the condition variable rather than the leader seeing pipe
> EOF, so ^C hits an extra pg_fatal("a worker process died unexpectedly")
> in the leader-- and your redirect swallows that one too. I put your

Excellent, and 100% agreed. No need for pipes/sockets and clunky
protocols if the fork() path is gone. I had already made a start on
exactly same idea, because I hadn't seen your thread (sorry). I have
a few comments/thoughts (short version: can we make a reusable generic
thread pool component for that?). I will look more closely at your
patches and comment on your thread.

> patch on top of that series and hit ^C on a 2.3 GB dump repeatedly at -j
> 2, 4, 8, and 100: clean exit every time, "terminated by user", nothing
> else. pg_stat_activity right afterward showed no live COPYs, up through
> -j 100-- so the PQcancel()s land and the backends actually stop, which
> TerminateThread never did. So your change composes with that work.

Interesting.