From 4d2a844242dcfb34e05dd0d880b1a283a514b16b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 28 Sep 2022 11:23:14 -0400 Subject: [PATCH] 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 --- src/backend/access/transam/xact.c | 14 ++++++++++++-- src/backend/utils/resowner/resowner.c | 7 ++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 2bb975943cf..c1ffbd89b88 100644 --- 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 ece5d98261a..37b43ee1f8c 100644 --- 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; }