aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael Paquier <michael@paquier.xyz>2024-06-28 12:31:29 +0900
committerMichael Paquier <michael@paquier.xyz>2024-06-28 12:31:29 +0900
commitd85fc4be11b38afd6d3abb586a6799299ed29470 (patch)
tree450bed4b8be43c075955fcbaafde0aaf9e10049c
parent4a7f91b3d3141e8898211a5b145b3c210b05c288 (diff)
downloadpostgresql-d85fc4be11b38afd6d3abb586a6799299ed29470.tar.gz
postgresql-d85fc4be11b38afd6d3abb586a6799299ed29470.zip
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
-rw-r--r--src/backend/utils/misc/injection_point.c8
1 files 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");