Allow callback functions to deregister themselves during a call.

Fetch the next-item pointer before the call not after, so that
we aren't dereferencing a dangling pointer if the callback
deregistered itself during the call.  The risky coding pattern
appears in CallXactCallbacks, CallSubXactCallbacks, and
ResourceOwnerReleaseInternal.  (There are some other places that
might be at hazard if they offered deregistration functionality,
but they don't.)

I (tgl) considered back-patching this, but desisted because it
wouldn't be very safe for extensions to rely on this working in
pre-v16 branches.

Hao Wu

Discussion: https://postgr.es/m/CAH+9SWXTiERkmhRke+QCcc+jRH8d5fFHTxh8ZK0-Yn4BSpyaAg@mail.gmail.com

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 2bb975943cf0a5591c9d627ba7358a8ed6e3a45d..c1ffbd89b883fb0d1f2cd02da016e3b7553d8380 100644 (file)
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -3656,9 +3656,14 @@ static void
 CallXactCallbacks(XactEvent event)
 {
    XactCallbackItem *item;
+   XactCallbackItem *next;
 
-   for (item = Xact_callbacks; item; item = item->next)
+   for (item = Xact_callbacks; item; item = next)
+   {
+       /* allow callbacks to unregister themselves when called */
+       next = item->next;
        item->callback(event, item->arg);
+   }
 }
 
 
@@ -3713,9 +3718,14 @@ CallSubXactCallbacks(SubXactEvent event,
                     SubTransactionId parentSubid)
 {
    SubXactCallbackItem *item;
+   SubXactCallbackItem *next;
 
-   for (item = SubXact_callbacks; item; item = item->next)
+   for (item = SubXact_callbacks; item; item = next)
+   {
+       /* allow callbacks to unregister themselves when called */
+       next = item->next;
        item->callback(event, mySubid, parentSubid, item->arg);
+   }
 }
 
 

diff --git a/src/backend/utils/resowner/resowner.c b/src/backend/utils/resowner/resowner.c
index ece5d98261ac75c5bac86fcd8ed939f28cf20983..37b43ee1f8c66c63074d89c0f2c2803eca5fa301 100644 (file)
--- a/src/backend/utils/resowner/resowner.c
+++ b/src/backend/utils/resowner/resowner.c
@@ -501,6 +501,7 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
    ResourceOwner child;
    ResourceOwner save;
    ResourceReleaseCallbackItem *item;
+   ResourceReleaseCallbackItem *next;
    Datum       foundres;
 
    /* Recurse to handle descendants */
@@ -701,8 +702,12 @@ ResourceOwnerReleaseInternal(ResourceOwner owner,
    }
 
    /* Let add-on modules get a chance too */
-   for (item = ResourceRelease_callbacks; item; item = item->next)
+   for (item = ResourceRelease_callbacks; item; item = next)
+   {
+       /* allow callbacks to unregister themselves when called */
+       next = item->next;
        item->callback(phase, isCommit, isTopLevel, item->arg);
+   }
 
    CurrentResourceOwner = save;
 }