PgLwLock breaks postgres reinitialization after backend crash

[JelteF]

Normally when a backend crashes with a segfault, postmaster will kill all other backends and then reinitialize everything. But this breaks when using PgLwLock, because shared memory initialization of the extension is executed again and then PgLwLock complains that it cannot attach.

2023-07-21 16:08:56.754 CEST [538131] LOG:  background worker "Some background worker" (PID 538137) was terminated by signal 11: Segmentation fault
2023-07-21 16:08:56.755 CEST [538131] LOG:  all server processes terminated; reinitializing
2023-07-21 16:08:56.924 CEST [538131] FATAL:  Can't attach, lock is not in an empty state: PgLwLockInner

[JelteF]

Easiest way to reproduce is to run the shmem example (which requires putting shmem in shared_preload_libraries). Then kill -11 <random pg backend>.

[JelteF]

Backtrace in case its helpful:

2023-07-21 16:40:02.995 CEST [550346] LOG:  all server processes terminated; reinitializing
2023-07-21 16:40:03.125 CEST [550346] FATAL:  Can't attach, lock is not in an empty state: PgLwLockInner
2023-07-21 16:40:03.125 CEST [550346] DETAIL:
       0: pgrx_pg_sys::submodules::panic::register_pg_guard_panic_hook::{{closure}}::{{closure}}
                 at /home/jelte/opensource/pgrx/pgrx-pg-sys/src/submodules/panic.rs:311:39
       1: std::thread::local::LocalKey<T>::try_with
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/thread/local.rs:270:16
       2: std::thread::local::LocalKey<T>::with
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/thread/local.rs:246:9
       3: pgrx_pg_sys::submodules::panic::register_pg_guard_panic_hook::{{closure}}
                 at /home/jelte/opensource/pgrx/pgrx-pg-sys/src/submodules/panic.rs:308:9
       4: <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/alloc/src/boxed.rs:1999:9
       5: std::panicking::rust_panic_with_hook
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:709:13
       6: std::panicking::begin_panic_handler::{{closure}}
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:597:13
       7: std::sys_common::backtrace::__rust_end_short_backtrace
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/sys_common/backtrace.rs:151:18
       8: rust_begin_unwind
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:593:5
       9: core::panicking::panic_fmt
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/panicking.rs:67:14
      10: core::result::unwrap_failed
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/result.rs:1651:5
      11: core::result::Result<T,E>::expect
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/core/src/result.rs:1033:23
      12: pgrx::lwlock::PgLwLock<T>::attach
                 at /home/jelte/opensource/pgrx/pgrx/src/lwlock.rs:80:9
      13: pgrx::shmem::PgSharedMem::shmem_init_locked
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:184:13
      14: <pgrx::lwlock::PgLwLock<T> as pgrx::shmem::PgSharedMemoryInitialization>::shmem_init
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:132:9
      15: shmem::_PG_init::_PG_init_inner::__pgrx_private_shmem_hook::__pgrx_private_shmem_hook_inner
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:68:34
      16: shmem::_PG_init::_PG_init_inner::__pgrx_private_shmem_hook::{{closure}}
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:61:13
      17: std::panicking::try::do_call
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:500:40
      18: __rust_try
      19: std::panicking::try
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:464:19
      20: std::panic::catch_unwind
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panic.rs:142:14
      21: pgrx_pg_sys::submodules::panic::run_guarded
                 at /home/jelte/opensource/pgrx/pgrx-pg-sys/src/submodules/panic.rs:407:11
      22: pgrx_pg_sys::submodules::panic::pgrx_extern_c_guard
                 at /home/jelte/opensource/pgrx/pgrx-pg-sys/src/submodules/panic.rs:384:11
      23: shmem::_PG_init::_PG_init_inner::__pgrx_private_shmem_hook
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:62:24
      24: shmem::_PG_init::_PG_init_inner::__pgrx_private_shmem_hook::__pgrx_private_shmem_hook_inner
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:65:25
      25: shmem::_PG_init::_PG_init_inner::__pgrx_private_shmem_hook::{{closure}}
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:61:13
      26: std::panicking::try::do_call
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:500:40
      27: __rust_try
      28: std::panicking::try
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:464:19
      29: std::panic::catch_unwind
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panic.rs:142:14
      30: pgrx_pg_sys::submodules::panic::run_guarded
                 at /home/jelte/opensource/pgrx/pgrx-pg-sys/src/submodules/panic.rs:407:11
      31: pgrx_pg_sys::submodules::panic::pgrx_extern_c_guard
                 at /home/jelte/opensource/pgrx/pgrx-pg-sys/src/submodules/panic.rs:384:11
      32: shmem::_PG_init::_PG_init_inner::__pgrx_private_shmem_hook
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:62:24
      33: shmem::_PG_init::_PG_init_inner::__pgrx_private_shmem_hook::__pgrx_private_shmem_hook_inner
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:65:25
      34: shmem::_PG_init::_PG_init_inner::__pgrx_private_shmem_hook::{{closure}}
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:61:13
      35: std::panicking::try::do_call
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:500:40
      36: __rust_try
      37: std::panicking::try
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:464:19
      38: std::panic::catch_unwind
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panic.rs:142:14
      39: pgrx_pg_sys::submodules::panic::run_guarded
                 at /home/jelte/opensource/pgrx/pgrx-pg-sys/src/submodules/panic.rs:407:11
      40: pgrx_pg_sys::submodules::panic::pgrx_extern_c_guard
                 at /home/jelte/opensource/pgrx/pgrx-pg-sys/src/submodules/panic.rs:384:11
      41: shmem::_PG_init::_PG_init_inner::__pgrx_private_shmem_hook
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:62:24
      42: shmem::_PG_init::_PG_init_inner::__pgrx_private_shmem_hook::__pgrx_private_shmem_hook_inner
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:65:25
      43: shmem::_PG_init::_PG_init_inner::__pgrx_private_shmem_hook::{{closure}}
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:61:13
      44: std::panicking::try::do_call
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:500:40
      45: __rust_try
      46: std::panicking::try
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:464:19
      47: std::panic::catch_unwind
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panic.rs:142:14
      48: pgrx_pg_sys::submodules::panic::run_guarded
                 at /home/jelte/opensource/pgrx/pgrx-pg-sys/src/submodules/panic.rs:407:11
      49: pgrx_pg_sys::submodules::panic::pgrx_extern_c_guard
                 at /home/jelte/opensource/pgrx/pgrx-pg-sys/src/submodules/panic.rs:384:11
      50: shmem::_PG_init::_PG_init_inner::__pgrx_private_shmem_hook
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:62:24
      51: shmem::_PG_init::_PG_init_inner::__pgrx_private_shmem_hook::__pgrx_private_shmem_hook_inner
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:65:25
      52: shmem::_PG_init::_PG_init_inner::__pgrx_private_shmem_hook::{{closure}}
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:61:13
      53: std::panicking::try::do_call
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:500:40
      54: __rust_try
      55: std::panicking::try
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panicking.rs:464:19
      56: std::panic::catch_unwind
                 at /rustc/8ede3aae28fe6e4d52b38157d7bfe0d3bceef225/library/std/src/panic.rs:142:14
      57: pgrx_pg_sys::submodules::panic::run_guarded
                 at /home/jelte/opensource/pgrx/pgrx-pg-sys/src/submodules/panic.rs:407:11
      58: pgrx_pg_sys::submodules::panic::pgrx_extern_c_guard
                 at /home/jelte/opensource/pgrx/pgrx-pg-sys/src/submodules/panic.rs:384:11
      59: shmem::_PG_init::_PG_init_inner::__pgrx_private_shmem_hook
                 at /home/jelte/opensource/pgrx/pgrx/src/shmem.rs:62:24
      60: CreateSharedMemoryAndSemaphores
                 at /home/jelte/.pgrx/13.11/src/backend/storage/ipc/ipci.c:284:3
      61: reset_shared
                 at /home/jelte/.pgrx/13.11/src/backend/postmaster/postmaster.c:2697:2
      62: PostmasterStateMachine
                 at /home/jelte/.pgrx/13.11/src/backend/postmaster/postmaster.c:4041:3
      63: reaper
                 at /home/jelte/.pgrx/13.11/src/backend/postmaster/postmaster.c:3264:2
      64: <unknown>
      65: __GI___select
                 at ./misc/../sysdeps/unix/sysv/linux/select.c:69:11
      66: pg_usleep
                 at /home/jelte/.pgrx/13.11/src/port/pgsleep.c:56:10
      67: ServerLoop
                 at /home/jelte/.pgrx/13.11/src/backend/postmaster/postmaster.c:1688:4
      68: PostmasterMain
                 at /home/jelte/.pgrx/13.11/src/backend/postmaster/postmaster.c:1412:11
      69: main
                 at /home/jelte/.pgrx/13.11/src/backend/main/main.c:210:3
      70: __libc_start_call_main
                 at ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
      71: __libc_start_main_impl
                 at ./csu/../csu/libc-start.c:392:3

[JelteF]

Based on my reading of the code for PgLwLock and OnceCell, it seems that PgLwLock should not be using OnceCell because postgres its reinitialization after a crash should reinitialize the lock, and OnceCell cannot be reinitialized. I think using UnsafeCell directly in PgLwLock should be able to solve this. I guess the question then becomes though what are the safety considerations, e.g. should pg_shmem_init be made unsafe and document that it's only safe to call from _PG_init, and it should not be called multiple times?

[thomcc]

That seems reasonable to me, but @eeeebbbbrrrr might have a better idea for how this should work.

[workingjubilee]

Should probably just use one of the other cell types or something like RwLock, no need to drop to UnsafeCell.

[JelteF]

Should probably just use one of the other cell types or something like RwLock, no need to drop to UnsafeCell.

Maybe I'm missing something/misunderstanding what you mean. But afaict using a cell other than UnsafeCell would require putting the RwLock itself in some other synchronization primitive (like Mutex/RwLock). That seems like quite a waste, because for any reasonable use of PgLwLock, Postgres will initialize it before any reads happen.

[thomcc]

Yeah, I think we'd need another synchronization primitive instead of a RefCell and doing that seems somewhat unfortunate.

[workingjubilee]

I have examined the atomics, shmem, and lwlock code and have come to a conclusion that it needs further, deeper reanalysis rather than just a quick hotfix here, unfortunately.

There are many SAFETY comments which prompt me to think something like:

https://www.youtube.com/watch?v=oHC1230OpOg