Re: Weird failure with latches in curculio on v15
Thomas Munro <thomas.munro@gmail.com>
From: Thomas Munro <thomas.munro@gmail.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Andres Freund <andres@anarazel.de>, Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Fujii Masao <fujii@postgresql.org>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-02-22T08:48:10Z
Lists: pgsql-hackers
On Tue, Feb 21, 2023 at 5:50 PM Nathan Bossart <nathandbossart@gmail.com> wrote: > On Tue, Feb 21, 2023 at 09:03:27AM +0900, Michael Paquier wrote: > > Perhaps beginning a new thread with a patch and a summary would be > > better at this stage? Another thing I am wondering is if it could be > > possible to test that rather reliably. I have been playing with a few > > scenarios like holding the system() call for a bit with hardcoded > > sleep()s, without much success. I'll try harder on that part.. It's > > been mentioned as well that we could just move away from system() in > > the long-term. > > I'm happy to create a new thread if needed, but I can't tell if there is > any interest in this stopgap/back-branch fix. Perhaps we should just jump > straight to the long-term fix that Thomas is looking into. Unfortunately the latch-friendly subprocess module proposal I was talking about would be for 17. I may post a thread fairly soon with design ideas + list of problems and decision points as I see them, and hopefully some sketch code, but it won't be a proposal for [/me checks calendar] next week's commitfest and probably wouldn't be appropriate in a final commitfest anyway, and I also have some other existing stuff to clear first. So please do continue with the stopgap ideas. BTW Here's an idea (untested) about how to reproduce the problem. You could copy the source from a system() implementation, call it doomed_system(), and insert kill(-getppid(), SIGQUIT) in between sigprocmask(SIG_SETMASK, &omask, NULL) and exec*(). Parent and self will handle the signal and both reach the proc_exit(). The systems that failed are running code like this: https://github.com/openbsd/src/blob/master/lib/libc/stdlib/system.c https://github.com/DragonFlyBSD/DragonFlyBSD/blob/master/lib/libc/stdlib/system.c I'm pretty sure these other implementations could fail in just the same way (they restore the handler before unblocking, so can run it just before exec() replaces the image): https://github.com/freebsd/freebsd-src/blob/main/lib/libc/stdlib/system.c https://github.com/lattera/glibc/blob/master/sysdeps/posix/system.c The glibc one is a bit busier and, huh, has a lock (I guess maybe deadlockable if proc_exit() also calls system(), but hopefully it doesn't), and uses fork() instead of vfork() but I don't think that's a material difference here (with fork(), parent and child run concurrently, while with vfork() the parent is suspended until the child exists or execs, and then processes its pending signals).
Commits
-
Avoid calling proc_exit() in processes forked by system().
- d0e7f95b4845 11.22 landed
- e2e16904224a 12.17 landed
- ac1dfc303d0e 13.13 landed
- 54fc9dca5b10 14.10 landed
- c9265ae80b6a 15.5 landed
- ee06199fcb0a 16.1 landed
- 97550c071197 17.0 landed
-
Move extra code out of the Pre/PostRestoreCommand() section.
- 882e522d6468 15.5 landed
- d1c56ad37b96 16.1 landed
- 8fb13dd6ab5b 17.0 landed
-
Revert refactoring of restore command code to shell_restore.c
- 2f6e15ac93c5 16.0 landed
-
Refactor code in charge of running shell-based recovery commands
- 9a740f81eb02 16.0 cited
-
Clean up inconsistent use of fflush().
- 7fed801135ba 16.0 cited
-
Report wait events for local shell commands like archive_command.
- 1b06d7bac901 15.0 cited