Re: Missing calls to UnlockBuffers() - unify error handling?

Hi,

On 2025-11-06 10:34:58 -0500, Andres Freund wrote:
> While hacking on a patch to "inline" the lock implementation of content locks
> into storage/buffer/, I looked at the callers of UnlockBuffers() and compared
> them to the callers of LWLockReleaseAll().
>
> Somewhat disconcertingly, there are more callers of LWLockReleaseAll(),
> without - afaict - analysis whether that's right.

A related issue I just found is that ProcKill() calls
SyncRepCleanupAtProcExit() before LWLockReleaseAll(). Which, given that
LWLockReleaseAll() uses lwlocks, doesn't seem great.

This may have kinda been ok in the past, as we'd normally have gone through
AbortTransaction() by that point. However, that's e.g. not true in background
workers, which just rely on ProcKill():

/*
* Do we need more cleanup here? For shmem-connected bgworkers, we
* will call InitProcess below, which will install ProcKill as exit
* callback. That will take care of releasing locks, etc.
*/

Seems we should just reorder the sequence in ProcKill() to first call
LWLockReleaseAll().

Greetings,

Andres Freund