Re: Injection point locking

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2024-06-25T02:14:57Z
Lists: pgsql-hackers
On Mon, Jun 24, 2024 at 01:29:38PM +0300, Heikki Linnakangas wrote:
> InjectionPointRun() acquires InjectionPointLock, looks up the hash entry,
> and releases the lock:
> 
> > 	LWLockAcquire(InjectionPointLock, LW_SHARED);
> > 	entry_by_name = (InjectionPointEntry *)
> > 		hash_search(InjectionPointHash, name,
> > 					HASH_FIND, &found);
> > 	LWLockRelease(InjectionPointLock);
> 
> Later, it reads fields from the entry it looked up:
> 
> > 		/* not found in local cache, so load and register */
> > 		snprintf(path, MAXPGPATH, "%s/%s%s", pkglib_path,
> > 				 entry_by_name->library, DLSUFFIX);
> 
> Isn't that a straightforward race condition, if the injection point is
> detached in between?

This is a feature, not a bug :)

Jokes apart, this is a behavior that Noah was looking for so as it is
possible to detach a point to emulate what a debugger would do with a
breakpoint for some of his tests with concurrent DDL bugs, so not
taking a lock while running a point is important.  It's true, though,
that we could always delay the LWLock release once the local cache is
loaded, but would it really matter?
--
Michael

Commits

  1. Improve locking around InjectionPointRun()