Hi,

While reviewing an extension that uses PGXN's BGWorkers, I noticed a potential unsoundness issue in the BGWorker systems:

When BackgroundWorker::attach_signal_handlers() is called with SignalWakeFlags::SIGHUP, then the handler for sighup can be executed at any arbitrary point in time. This is normal.

However, the installed handler for sighup is worker_spi_sighup, which in its handling calls pg_sys::ProcessConfigFile(pg_sys::GucContext::PGC_SIGHUP), which will update the current GUC state machine. This is unsafe, because it interrupts the normal execution flow to start work on replacing GUCs, potentially ones that are currently being used. This is why in normal PostgreSQL backends the handling of the actual config file changes is handled by CHECK_FOR_INTERUPTS after SIGHUP was signaled, handled by SignalHandlerForConfigReload, which then set ConfigReloadPending = true and SetLatch(MyLatch).

The handling of BGWorker interrupts instead seems to allow updates to the GUCs at any point in time (not just specific code locations marked with CHECK_FOR_INTERRUPTS), invalidating the by-ref -type GUCs at arbitrary points in time, making any dependency on GUC reloading with SIGHUP in PGRX-based backgroundworkers fundamentally unsafe (and, given ProcessConfigFile's use of various other systems, probably things like MemoryContext and errno handling, too).

Example:

let setting = GucSetting::new(Some(c"text"));

...
pub fn my_code() {
  let value = GucSetting::get().unwrap()?;
  assert!(value.len() > 0);
  # SIGHUP happens, with changes to the GucSetting, potentially deallocating the previous value
  let chr = value[0];
}

This, plus similar accesses of GUCs in BGWorker seems totally unsafe when the default pgxn signal handler infra is used for SIGHUPs. Even copying the string for later use (to protect against this GUC system -based deallocation) is not safe, as the value can be deallocated while we're still copying the old value.