Fix race in background worker termination for database

Hi Li-san

Thank you for sharing this fix patch.

BackendPidGetProc() returns a PGPROC pointer after releasing
ProcArrayLock, so the returned PGPROC is no longer protected. The ?
corresponding entry in allProcs could be recycled after the function
returns, in which case proc could refer to a different process when proc-

databaseId is accessed.

I'm not sure how long the PGPROC might be reused or modified after it is
retrieved, but it's true that the scenario you're describing could occur
depending on the timing.

I think this fix looks good. I would appreciate hearing what the other members think.

Regards,
Aya Iwata

On Fri, Jul 24, 2026 at 11:31 AM Chao Li <li.evan.chao@gmail.com> wrote:

Hi,

While reading code for another work, I spotted something suspicious that
might be an oversight in “[f1e251be8] Allow bgworkers to be terminated for
database-related commands”.

In TerminateBackgroundWorkersForDatabase(Oid databaseId)
```
/*
* Iterate through slots, looking for workers connected to the
given
* database.
*/
for (int slotno = 0; slotno < BackgroundWorkerData->total_slots;
slotno++)
{
BackgroundWorkerSlot *slot =
&BackgroundWorkerData->slot[slotno];

if (slot->in_use &&
(slot->worker.bgw_flags & BGWORKER_INTERRUPTIBLE))
{
PGPROC *proc = BackendPidGetProc(slot->pid);

if (proc && proc->databaseId == databaseId)
{
slot->terminate = true;
signal_postmaster = true;

elog(DEBUG1, "termination requested for
worker (PID %d) on database %u",
(int) slot->pid, databaseId);
}
}
}
```

BackendPidGetProc() returns a PGPROC pointer after releasing
ProcArrayLock, so the returned PGPROC is no longer protected. The
corresponding entry in allProcs could be recycled after the function
returns, in which case proc could refer to a different process when
proc->databaseId is accessed.

Similarly, the postmaster may update slot->pid between the lookup and the
elog(), so the logged PID has a small race as well.

I don’t have a concrete repro for this, but I think we should hold
ProcArrayLock and use BackendPidGetProcWithLock() instead. See the attached
patch for the changes.

One thing to point out is that, to keep the lock section as small as
possible, I added a local variable, terminate, so that elog() can be placed
outside the lock section. Maybe I’m overly cautious here, please let me
know if that is unnecessary.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

The fix looks correct to me. BackendPidGetProc() releases ProcArrayLock

before returning, so the returned PGPROC may be removed from the proc array
and reused before its databaseId is examined. Keeping ProcArrayLock held
across both BackendPidGetProcWithLock() and the databaseId check closes
that race.

I suggest a small changes:

Copy slot->pid before acquiring ProcArrayLock, since ProcArrayLock does not

protect the background worker slot and there is no reason to include that
read

in the ProcArrayLock critical section.

Regards,
Haibo

On Fri, Jul 24, 2026 at 01:32:05PM +0000, Aya Iwata (Fujitsu) wrote:

I'm not sure how long the PGPROC might be reused or modified after it is
retrieved, but it's true that the scenario you're describing could occur
depending on the timing.

I think this fix looks good. I would appreciate hearing what the other members think.

Hmm. The case of an interruptible bgworker terminating itself between
the moment BackendPidGetProc() is called and the moment we read the
databaseId, meaning that the PGPROC entry gets cleaned with the same
proc entry being reused afterwards? Say (not tested myself, just
hand-waving for now):
- Call BackendPidGetProc()
- Hold a breakpoint with the proc pointer to enlarge the window.
- Manipulate the PGPROC slots and change a databaseId.
- Terminate incorrect database.

That would warrant a WithLock() call with a procarray lock held until
we check the contents of the procarray entry. Keeping the DEBUG2
inside or outside the LWLock does not matter much, that's a LWLock we
talk about, not a spinlock.
--
Michael

On Jul 25, 2026, at 07:10, Michael Paquier <michael@paquier.xyz> wrote:

On Fri, Jul 24, 2026 at 01:32:05PM +0000, Aya Iwata (Fujitsu) wrote:

I'm not sure how long the PGPROC might be reused or modified after it is
retrieved, but it's true that the scenario you're describing could occur
depending on the timing.

I think this fix looks good. I would appreciate hearing what the other members think.

Hmm. The case of an interruptible bgworker terminating itself between
the moment BackendPidGetProc() is called and the moment we read the
databaseId, meaning that the PGPROC entry gets cleaned with the same
proc entry being reused afterwards? Say (not tested myself, just
hand-waving for now):
- Call BackendPidGetProc()
- Hold a breakpoint with the proc pointer to enlarge the window.
- Manipulate the PGPROC slots and change a databaseId.
- Terminate incorrect database.

That would warrant a WithLock() call with a procarray lock held until
we check the contents of the procarray entry.

Thanks for confirming.

Keeping the DEBUG2
inside or outside the LWLock does not matter much, that's a LWLock we
talk about, not a spinlock.
--
Michael

Okay, I was being overly cautious.

PFA v2:

* Moved elog() into the lock section and removed the now-unneeded local variable “terminate".
* Incorporated Haibo’s suggestion to copy slot->pid before acquiring the lock.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/