From 8377dadf7708ee1939c55477523422594ddf304e Mon Sep 17 00:00:00 2001 From: Hayato Kuroda Date: Wed, 11 Nov 2026 17:59:15 +0900 Subject: [PATCH v2 2/2] injection points: Add handles for detaching callbacks The injection_points test module tracks injection points attached locally so that it can detach them when the owning backend exits. This cleanup previously identified an injection point only by name. If another backend detached the point and attached a new callback with the same name before the original backend exited, the cleanup could detach the new callback instead. Add an InjectionPointHandle that identifies the shared-memory slot and generation used by a particular call to InjectionPointAttachWithHandle(). InjectionPointDetachByHandle() detaches the callback only while that handle still matches, so a stale cleanup request cannot affect a callback attached later. Keep the existing name-based attach and detach interfaces unchanged. Use the new interface to track locally attached injection points in the test module, and document the new functions. --- doc/src/sgml/xfunc.sgml | 28 +++++ src/backend/utils/misc/injection_point.c | 114 +++++++++++++++--- src/include/utils/injection_point.h | 16 +++ .../injection_points/injection_points.c | 60 ++++++--- src/tools/pgindent/typedefs.list | 2 + 5 files changed, 181 insertions(+), 39 deletions(-) diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index bc04d9dfc13..144a7813113 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3940,6 +3940,22 @@ extern void InjectionPointAttach(const char *name, given as argument to the callback when executed. + + To attach a callback and obtain a handle for detaching it later, use: + +extern void InjectionPointAttachWithHandle(const char *name, + const char *library, + const char *function, + const void *private_data, + int private_data_size, + InjectionPointHandle *handle); + + This attaches the callback in the same way as + InjectionPointAttach and initializes + handle. The handle can be used to detach only + the callback attached by this call. + + Here is an example of callback for InjectionPointCallback: @@ -3995,6 +4011,18 @@ extern bool InjectionPointDetach(const char *name); otherwise. + + The handle initialized by + InjectionPointAttachWithHandle can be passed to: + +extern bool InjectionPointDetachByHandle(const InjectionPointHandle *handle); + + This detaches only the callback attached by the corresponding + InjectionPointAttachWithHandle call. If that callback + has already been detached, it returns false without + affecting a callback subsequently attached to the injection point. + + A callback attached to an injection point is available across all the backends including the backends started after diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c index 603ad484d64..58b5715ec1e 100644 --- a/src/backend/utils/misc/injection_point.c +++ b/src/backend/utils/misc/injection_point.c @@ -112,6 +112,7 @@ static HTAB *InjectionPointCache = NULL; static void InjectionPointShmemRequest(void *arg); static void InjectionPointShmemInit(void *arg); +static void InjectionPointDetachSlot(int slot_idx, uint64 generation); /* * injection_point_cache_add @@ -253,17 +254,49 @@ InjectionPointShmemInit(void *arg) for (int i = 0; i < MAX_INJECTION_POINTS; i++) pg_atomic_init_u64(&ActiveInjectionPoints->entries[i].generation, 0); } + +/* + * Mark an injection point slot as unused, and update max_inuse if needed. + */ +static void +InjectionPointDetachSlot(int slot_idx, uint64 generation) +{ + InjectionPointEntry *entry; + int max_inuse; + + Assert(generation % 2 != 0); + Assert(LWLockHeldByMeInMode(InjectionPointLock, LW_EXCLUSIVE)); + + entry = &ActiveInjectionPoints->entries[slot_idx]; + Assert(pg_atomic_read_u64(&entry->generation) == generation); + pg_atomic_write_u64(&entry->generation, generation + 1); + + /* If we just removed the highest-numbered entry, update 'max_inuse' */ + max_inuse = (int) pg_atomic_read_u32(&ActiveInjectionPoints->max_inuse); + if (slot_idx == max_inuse - 1) + { + for (; slot_idx >= 0; --slot_idx) + { + entry = &ActiveInjectionPoints->entries[slot_idx]; + generation = pg_atomic_read_u64(&entry->generation); + if (generation % 2 != 0) + break; + } + pg_atomic_write_u32(&ActiveInjectionPoints->max_inuse, slot_idx + 1); + } +} #endif /* USE_INJECTION_POINTS */ /* * Attach a new injection point. */ void -InjectionPointAttach(const char *name, - const char *library, - const char *function, - const void *private_data, - int private_data_size) +InjectionPointAttachWithHandle(const char *name, + const char *library, + const char *function, + const void *private_data, + int private_data_size, + InjectionPointHandle *handle) { #ifdef USE_INJECTION_POINTS InjectionPointEntry *entry; @@ -334,11 +367,31 @@ InjectionPointAttach(const char *name, LWLockRelease(InjectionPointLock); + /* + * Return a handle identifying this generation of the slot, if requested. + */ + if (handle) + { + handle->slot_idx = free_idx; + handle->generation = generation + 1; + } + #else elog(ERROR, "injection points are not supported by this build"); #endif } +void +InjectionPointAttach(const char *name, + const char *library, + const char *function, + const void *private_data, + int private_data_size) +{ + InjectionPointAttachWithHandle(name, library, function, private_data, + private_data_size, NULL); +} + /* * Detach an existing injection point. * @@ -369,28 +422,51 @@ InjectionPointDetach(const char *name) { Assert(!found); found = true; - pg_atomic_write_u64(&entry->generation, generation + 1); + InjectionPointDetachSlot(idx, generation); break; } } + LWLockRelease(InjectionPointLock); - /* If we just removed the highest-numbered entry, update 'max_inuse' */ - if (found && idx == max_inuse - 1) - { - for (; idx >= 0; --idx) - { - InjectionPointEntry *entry = &ActiveInjectionPoints->entries[idx]; - uint64 generation; + return found; +#else + elog(ERROR, "Injection points are not supported by this build"); + return true; /* silence compiler */ +#endif +} - generation = pg_atomic_read_u64(&entry->generation); - if (generation % 2 != 0) - break; - } - pg_atomic_write_u32(&ActiveInjectionPoints->max_inuse, idx + 1); +/* + * Detach the injection point identified by the given handle. + * + * Returns false if the slot has already been detached or reused since the + * handle was initialized. + */ +bool +InjectionPointDetachByHandle(const InjectionPointHandle *handle) +{ +#ifdef USE_INJECTION_POINTS + InjectionPointEntry *entry; + uint64 generation; + bool found = false; + int slot_idx; + + Assert(handle); + + LWLockAcquire(InjectionPointLock, LW_EXCLUSIVE); + + slot_idx = handle->slot_idx; + entry = &ActiveInjectionPoints->entries[slot_idx]; + generation = pg_atomic_read_u64(&entry->generation); + + if (generation == handle->generation) + { + found = true; + InjectionPointDetachSlot(slot_idx, generation); } - LWLockRelease(InjectionPointLock); + LWLockRelease(InjectionPointLock); return found; + #else elog(ERROR, "Injection points are not supported by this build"); return true; /* silence compiler */ diff --git a/src/include/utils/injection_point.h b/src/include/utils/injection_point.h index fabd1455c3c..b160271419e 100644 --- a/src/include/utils/injection_point.h +++ b/src/include/utils/injection_point.h @@ -24,6 +24,15 @@ typedef struct InjectionPointData const char *function; } InjectionPointData; +/* + * Identifies one generation of an injection point slot. + */ +typedef struct InjectionPointHandle +{ + int slot_idx; + uint64 generation; +} InjectionPointHandle; + /* * Injection points require --enable-injection-points. */ @@ -51,11 +60,18 @@ extern void InjectionPointAttach(const char *name, const char *function, const void *private_data, int private_data_size); +extern void InjectionPointAttachWithHandle(const char *name, + const char *library, + const char *function, + const void *private_data, + int private_data_size, + InjectionPointHandle *handle); extern void InjectionPointLoad(const char *name); extern void InjectionPointRun(const char *name, void *arg); extern void InjectionPointCached(const char *name, void *arg); extern bool IsInjectionPointAttached(const char *name); extern bool InjectionPointDetach(const char *name); +extern bool InjectionPointDetachByHandle(const InjectionPointHandle *handle); /* Get the current set of injection points attached */ extern List *InjectionPointList(void); diff --git a/src/test/modules/injection_points/injection_points.c b/src/test/modules/injection_points/injection_points.c index 5e1bbc2f5c9..564f86def01 100644 --- a/src/test/modules/injection_points/injection_points.c +++ b/src/test/modules/injection_points/injection_points.c @@ -46,11 +46,18 @@ PG_MODULE_MAGIC; #define INJ_WAIT_MAX_US 100000 /* 100ms */ /* - * List of injection points stored in TopMemoryContext attached - * locally to this process. + * List of injection points stored in TopMemoryContext attached locally to this + * process. Handles ensure that exit cleanup does not detach a different point + * that reused the same name or slot. */ static List *inj_list_local = NIL; +typedef struct LocalInjectionPoint +{ + char *name; + InjectionPointHandle handle; +} LocalInjectionPoint; + /* * Shared state information for injection points. * @@ -174,19 +181,13 @@ injection_point_allowed(const InjectionPointCondition *condition, static void injection_points_cleanup(int code, Datum arg) { - ListCell *lc; - /* Leave if nothing is tracked locally */ if (!injection_point_local) return; /* Detach all the local points */ - foreach(lc, inj_list_local) - { - char *name = strVal(lfirst(lc)); - - (void) InjectionPointDetach(name); - } + foreach_ptr(LocalInjectionPoint, local_inj, inj_list_local) + InjectionPointDetachByHandle(&local_inj->handle); } /* Set of callbacks available to be attached to an injection point. */ @@ -360,16 +361,29 @@ injection_points_attach(PG_FUNCTION_ARGS) strlcpy(condition.str, str, INJ_DATA_MAXLEN); } - InjectionPointAttach(name, "injection_points", function, &condition, - sizeof(InjectionPointCondition)); - - if (injection_point_local) + if (!injection_point_local) { + InjectionPointAttach(name, "injection_points", function, &condition, + sizeof(InjectionPointCondition)); + } + else + { + InjectionPointHandle handle = {0}; MemoryContext oldctx; + LocalInjectionPoint *local_inj; /* Local injection point, so track it for automated cleanup */ + InjectionPointAttachWithHandle(name, "injection_points", function, + &condition, + sizeof(InjectionPointCondition), + &handle); + oldctx = MemoryContextSwitchTo(TopMemoryContext); - inj_list_local = lappend(inj_list_local, makeString(pstrdup(name))); + local_inj = palloc_object(LocalInjectionPoint); + local_inj->name = pstrdup(name); + local_inj->handle = handle; + + inj_list_local = lappend(inj_list_local, local_inj); MemoryContextSwitchTo(oldctx); } @@ -552,11 +566,17 @@ injection_points_detach(PG_FUNCTION_ARGS) /* Remove point from local list, if required */ if (inj_list_local != NIL) { - MemoryContext oldctx; - - oldctx = MemoryContextSwitchTo(TopMemoryContext); - inj_list_local = list_delete(inj_list_local, makeString(name)); - MemoryContextSwitchTo(oldctx); + foreach_ptr(LocalInjectionPoint, local_inj, inj_list_local) + { + if (strcmp(local_inj->name, name) == 0) + { + inj_list_local = foreach_delete_current(inj_list_local, + local_inj); + pfree(local_inj->name); + pfree(local_inj); + break; + } + } } PG_RETURN_VOID(); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 8f65cee41b1..812ca7aff39 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1350,6 +1350,7 @@ InjectionPointCondition InjectionPointConditionType InjectionPointData InjectionPointEntry +InjectionPointHandle InjectionPointSharedState InjectionPointsCtl InlineCodeBlock @@ -1629,6 +1630,7 @@ ListenerEntry LoInfo LoadStmt LocalBufferLookupEnt +LocalInjectionPoint LocalPgBackendStatus LocalTransactionId Location -- 2.52.0