From d85fc4be11b38afd6d3abb586a6799299ed29470 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 28 Jun 2024 12:31:29 +0900 Subject: [PATCH] Improve locking around InjectionPointRun() As coded, an injection point could be loaded into the local cache without the LWLock InjectionPointLock taken, hence a point detached and re-attached concurrently of a point running calling InjectionPointRun() may finish by loading a callback it did no set initially. Based on all the cases discussed until now on the lists, it is fine to delay the lock release until the callback is run, so let's do that. While on it, remove a useless LWLockRelease() called before an error in InjectionPointAttach(). Per discussion with Heikki Linnakangas and Noah Misch. Discussion: https://postgr.es/m/e1ffb822-054e-4006-ac06-50532767f75b@iki.fi --- src/backend/utils/misc/injection_point.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c index 5c2a0d2297e..afae0dbedf4 100644 --- a/src/backend/utils/misc/injection_point.c +++ b/src/backend/utils/misc/injection_point.c @@ -234,10 +234,7 @@ InjectionPointAttach(const char *name, hash_search(InjectionPointHash, name, HASH_ENTER, &found); if (found) - { - LWLockRelease(InjectionPointLock); elog(ERROR, "injection point \"%s\" already defined", name); - } /* Save the entry */ strlcpy(entry_by_name->name, name, sizeof(entry_by_name->name)); @@ -300,7 +297,6 @@ InjectionPointRun(const char *name) entry_by_name = (InjectionPointEntry *) hash_search(InjectionPointHash, name, HASH_FIND, &found); - LWLockRelease(InjectionPointLock); /* * If not found, do nothing and remove it from the local cache if it @@ -309,6 +305,7 @@ InjectionPointRun(const char *name) if (!found) { injection_point_cache_remove(name); + LWLockRelease(InjectionPointLock); return; } @@ -343,6 +340,9 @@ InjectionPointRun(const char *name) /* Now loaded, so get it. */ injection_callback = injection_point_cache_get(name, &private_data); + + LWLockRelease(InjectionPointLock); + injection_callback(name, private_data); #else elog(ERROR, "Injection points are not supported by this build");