Install an error check into cancel_before_shmem_exit().

Historically, cancel_before_shmem_exit() just silently did nothing
if the specified callback wasn't the top-of-stack.  The folly of
ignoring this case was exposed by the bugs fixed in 303640199 and
bab150045, so let's make it throw elog(ERROR) instead.

There is a decent argument to be made that PG_ENSURE_ERROR_CLEANUP
should use some separate infrastructure, so it wouldn't break if
something inside the guarded code decides to register a new
before_shmem_exit callback.  However, a survey of the surviving
uses of before_shmem_exit() and PG_ENSURE_ERROR_CLEANUP doesn't
show any plausible conflicts of that sort today, so for now we'll
forgo the extra complexity.  (It will almost certainly become
necessary if anyone ever wants to wrap PG_ENSURE_ERROR_CLEANUP
around arbitrary user-defined actions, though.)

No backpatch, since this is developer support not a production issue.

Bharath Rupireddy, per advice from Andres Freund, Robert Haas, and myself

Discussion: https://postgr.es/m/CALj2ACWk7j4F2v2fxxYfrroOF=AdFNPr1WsV+AGtHAFQOqm_pw@mail.gmail.com

diff --git a/src/backend/storage/ipc/ipc.c b/src/backend/storage/ipc/ipc.c
index bdbc2c3ac4bc06c7f55bba340200d568cad56e83..11c3f132a100268eb411a4095f43a81fda3203b4 100644 (file)
--- a/src/backend/storage/ipc/ipc.c
+++ b/src/backend/storage/ipc/ipc.c
@@ -381,9 +381,9 @@ on_shmem_exit(pg_on_exit_callback function, Datum arg)
  *     cancel_before_shmem_exit
  *
  *     this function removes a previously-registered before_shmem_exit
- *     callback.  For simplicity, only the latest entry can be
- *     removed.  (We could work harder but there is no need for
- *     current uses.)
+ *     callback.  We only look at the latest entry for removal, as we
+ *         expect callers to add and remove temporary before_shmem_exit
+ *         callbacks in strict LIFO order.
  * ----------------------------------------------------------------
  */
 void
@@ -394,6 +394,9 @@ cancel_before_shmem_exit(pg_on_exit_callback function, Datum arg)
        == function &&
        before_shmem_exit_list[before_shmem_exit_index - 1].arg == arg)
        --before_shmem_exit_index;
+   else
+       elog(ERROR, "before_shmem_exit callback (%p,0x%llx) is not the latest entry",
+            function, (long long) arg);
 }
 
 /* ----------------------------------------------------------------