sharpetronics/PostgreSQL
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-21 00:02:53 -04:00
Code Issues Projects Releases Wiki Activity

Improve locking around InjectionPointRun()

Browse Source 
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
This commit is contained in:
Michael Paquier 2024-06-28 12:31:29 +09:00
parent 4a7f91b3d3
commit d85fc4be11
1 changed files with 4 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/backend/utils/misc/injection_point.c
View File

@ -234,10 +234,7 @@ InjectionPointAttach(const char *name,
hash_search(InjectionPointHash, name, hash_search(InjectionPointHash, name,
HASH_ENTER, &found); HASH_ENTER, &found);
if (found) if (found)
{
LWLockRelease(InjectionPointLock);
elog(ERROR, "injection point \"%s\" already defined", name); elog(ERROR, "injection point \"%s\" already defined", name);
}
/* Save the entry */ /* Save the entry */
strlcpy(entry_by_name->name, name, sizeof(entry_by_name->name)); strlcpy(entry_by_name->name, name, sizeof(entry_by_name->name));
@ -300,7 +297,6 @@ InjectionPointRun(const char *name)
entry_by_name = (InjectionPointEntry *) entry_by_name = (InjectionPointEntry *)
hash_search(InjectionPointHash, name, hash_search(InjectionPointHash, name,
HASH_FIND, &found); HASH_FIND, &found);
LWLockRelease(InjectionPointLock);
/* /*
* If not found, do nothing and remove it from the local cache if it * If not found, do nothing and remove it from the local cache if it
@ -309,6 +305,7 @@ InjectionPointRun(const char *name)
if (!found) if (!found)
{ {
injection_point_cache_remove(name); injection_point_cache_remove(name);
LWLockRelease(InjectionPointLock);
return; return;
} }
@ -343,6 +340,9 @@ InjectionPointRun(const char *name)
/* Now loaded, so get it. */ /* Now loaded, so get it. */
injection_callback = injection_point_cache_get(name, &private_data); injection_callback = injection_point_cache_get(name, &private_data);
LWLockRelease(InjectionPointLock);
injection_callback(name, private_data); injection_callback(name, private_data);
#else #else
elog(ERROR, "Injection points are not supported by this build"); elog(ERROR, "Injection points are not supported by this build");