Fix pgrx install causing postgresql coredump

[Sasasu]

this is because pgrx will overwrite the .so file in place.

dlopen will use MAP_PRIVATE to open the file and map a read-only memory use mmap(2). usually this memory has copy-on-write. if others is modifying the file. the previously mapped memory should not change.

but there is a undefined behavior, from man mmap(2):

MAP_PRIVATE: It is unspecified whether changes made to the file after the mmap()
call are visible in the mapped region.

what actually happens is that the read-only memory in postgresql is modified, and all pointers in the .TEXT segment is mashed up. the call stack looks like

 #0  0x0000000000731510 in ?? ()
 #1  0x00007f7a94515132 in core::sync::atomic::AtomicUsize::store (self=0x7f7a95110318 <pgrx_pg_sys::submodules::thread_check::ACTIVE_THREAD::h60448dcb81097e92>, val=0, order=core::sync::atomic::Ordering::Relaxed) at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/sync/atomic.rs:2291
 #2  0x00007f7a944f574b in pgrx_pg_sys::submodules::thread_check::init_active_thread::clear_in_child () at src/submodules/thread_check.rs:39
 #3  0x00007f7a962f8a88 in __run_postfork_handlers (who=who@entry=atfork_run_child, do_locking=do_locking@entry=false, lastrun=lastrun@entry=2) at register-atfork.c:187
 #4  0x00007f7a962df773 in __libc_fork () at fork.c:109
 #5  0x00005555e66ad948 in fork_process () at fork_process.c:61
 #6  0x00005555e66a5d48 in StartAutoVacWorker () at autovacuum.c:1543
 #7  0x00005555e66c43f7 in StartAutovacuumWorker () at postmaster.c:6155
 #8  0x00005555e66c3d21 in sigusr1_handler (postgres_signal_arg=10) at postmaster.c:5820
 #9  <signal handler called>
 #10 0x00007f7a9630eb84 in __GI___select (nfds=6, readfds=0x7ffc61c2fa40, writefds=0x0, exceptfds=0x0, timeout=0x7ffc61c2f9b0) at ../sysdeps/unix/sysv/linux/select.c:69
 #11 0x00005555e66be343 in ServerLoop () at postmaster.c:1950
 #12 0x00005555e66bdb0f in PostmasterMain (argc=5, argv=0x5555e8fb5490) at postmaster.c:1631
 #13 0x00005555e6560e41 in main (argc=5, argv=0x5555e8fb5490) at main.c:240

the is pgrx_pg_sys try to run the postfork hook. but the variable ACTIVE_THREAD and the code binary does not in the previous place.

[workingjubilee]

Wow that's cursed.

[workingjubilee]

Hmm, can you give more exact repro instructions?

This sounds like something we should be testing under valgrind, so if it's possible to replicate this consistently, jamming out a script to use this as a regression test would be nice.

[Sasasu]

1. run cargo pgrx install
2. set shared_preload_libraries = <the pgrx project>
3. reboot PostgreSQL to take effect
4. run cargo pgrx install, and wait for auto vacuum or create a new session.
5. PostgreSQL crash

sorry I forget to mention the repro need shared_preload_libraries

[Sasasu]

the key point is ensure libc::pthread_atfork(None, None, Some(clear_in_child)) is executed in postmaster. (call it in pg_init with shared_preload_libraries)

and try to add some static variable to the pgrx project between <3> and <4> if the .so has the same memory layout with the previous build

---

another repro is to write some simple UDF, and install it. load and run it.
then add more UDF, install, and run it again. if PostgreSQL does not reload .so file. the memory layout should be different. will crash when calling UDF or a wrong UDF gets executed.

---

or try pgml if a non-minimal repro is acceptable

[beeender]

run cargo pgrx install, and wait for auto vacuum or create a new session.

I think it can be reproduced by simply creating any new session, like psql xxx.

[Sasasu]

postgresml/postgresml#530

this person copies the .so file. and get crashed on every UDF call. after rebooting PostgreSQL it returns to normal.
he reproduced this bug, but not by pgrx install.

[eeeebbbbrrrr]

It seems to me we should just always do it this way, keeping the comments as to why. macos and linux are our two primary target platforms and if they both exhibit similar problems with overwriting the existing .so, I'm sure other platforms do too.

[Sasasu]

BTW, have you reproduced the problem?

I find install(1) will also remove the .so file before creating it. and PGXS is using install(1)

touch a b
strace -e newfstatat,openat,unlinkat,ioctl install a b  

openat(AT_FDCWD, "b", O_RDONLY|O_PATH|O_DIRECTORY) = -1 ENOTDIR (Not a directory)
newfstatat(AT_FDCWD, "a", {st_mode=S_IFREG|0644, st_size=4, ...}, 0) = 0
newfstatat(AT_FDCWD, "b", {st_mode=S_IFREG|0755, st_size=4, ...}, AT_SYMLINK_NOFOLLOW) = 0
unlinkat(AT_FDCWD, "b", 0)              = 0
openat(AT_FDCWD, "a", O_RDONLY)         = 3
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=4, ...}, AT_EMPTY_PATH) = 0
openat(AT_FDCWD, "b", O_WRONLY|O_CREAT|O_EXCL, 0600) = 4
ioctl(4, BTRFS_IOC_CLONE or FICLONE, 3) = 0

[workingjubilee]

That is exciting.

[workingjubilee]

2023-08-22 18:28:34.336 PDT [395747] LOG:  from bgworker: (Some("Hi"), Some(8), Some("(8,42)"))
2023-08-22 18:28:34.336 PDT [395747] LOG:  from bgworker: (Some("Hi"), Some(9), Some("(9,42)"))
2023-08-22 18:28:34.336 PDT [395747] LOG:  from bgworker: (Some("Hi"), Some(10), Some("(10,42)"))
2023-08-22 18:28:44.634 PDT [395740] LOG:  background worker "bgworkers" (PID 395747) was terminated by signal 11: Segmentation fault
2023-08-22 18:28:44.634 PDT [395740] LOG:  terminating any other active server processes
2023-08-22 18:28:44.733 PDT [395740] LOG:  statistics collector process (PID 395746) was terminated by signal 11: Segmentation fault
2023-08-22 18:28:44.733 PDT [395740] LOG:  all server processes terminated; reinitializing
2023-08-22 18:28:45.054 PDT [395740] LOG:  startup process (PID 398010) was terminated by signal 11: Segmentation fault
2023-08-22 18:28:45.054 PDT [395740] LOG:  aborting startup due to startup process failure
2023-08-22 18:28:45.069 PDT [395740] LOG:  database system is shut down

Successful repro.

There seems to have been more than just one segfault, even. Great. Excellent. Fantastic. Merging this and I will look into a regression test after we expand cargo-pgrx's capabilities a bit to make such easier.