Fix crash in autoprewarm.

Commit abb0b4fc03 moved the shared state for autoprewarm to a
dynamic shared memory (DSM) segment, but it left apw_detach_shmem()
in the on_shmem_exit callback list for the autoprewarm leader
process.  This is a problem because shmem_exit() detaches all the
DSM segments prior to calling the on_shmem_exit callbacks, thus
producing segfaults in the exit path for the autoprewarm leader
process.

To fix, move apw_detach_shmem() to the before_shmem_exit callback
list.  This commit also adds a check to pg_prewarm's test that the
server shut down normally.  It might be worth making this a common
check for all shutdowns in TAP tests, but that is left as a future
exercise.

Reported-by: Andres Freund
Reviewed-by: Andres Freund, Álvaro Herrera
Discussion: https://postgr.es/m/20240122204117.swton324xcoodnyi%40awork3.anarazel.de

diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c
index 9ea6c2252ad9231cec5721f955dd3042a9c11c6b..06ee21d49631112050d9594f512232de5c8172fa 100644 (file)
--- a/contrib/pg_prewarm/autoprewarm.c
+++ b/contrib/pg_prewarm/autoprewarm.c
@@ -164,8 +164,14 @@ autoprewarm_main(Datum main_arg)
    if (apw_init_shmem())
        first_time = false;
 
-   /* Set on-detach hook so that our PID will be cleared on exit. */
-   on_shmem_exit(apw_detach_shmem, 0);
+   /*
+    * Set on-detach hook so that our PID will be cleared on exit.
+    *
+    * NB: Autoprewarm's state is stored in a DSM segment, and DSM segments
+    * are detached before calling the on_shmem_exit callbacks, so we must put
+    * apw_detach_shmem in the before_shmem_exit callback list.
+    */
+   before_shmem_exit(apw_detach_shmem, 0);
 
    /*
     * Store our PID in the shared memory area --- unless there's already

diff --git a/contrib/pg_prewarm/t/001_basic.pl b/contrib/pg_prewarm/t/001_basic.pl
index bcd23a691466974ceef8513bfe13ed4d148645a7..825d3448ee5622939718d3fe2e0ba393044754f7 100644 (file)
--- a/contrib/pg_prewarm/t/001_basic.pl
+++ b/contrib/pg_prewarm/t/001_basic.pl
@@ -55,4 +55,10 @@ $node->wait_for_log(
 
 $node->stop;
 
+# control file should indicate normal shut down
+command_like(
+   [ 'pg_controldata', $node->data_dir() ],
+   qr/Database cluster state:\s*shut down/,
+   'cluster shut down normally');
+
 done_testing();