From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org, Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Subject: | EXEC_BACKEND vs bgworkers without BGWORKER_SHMEM_ACCESS |
Date: | 2021-08-02 06:51:16 |
Message-ID: | 20210802065116.j763tz3vz4egqy3w@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
While looking at a patch I noticed that SubPostmasterMain() for bgworkers
unconditionally does
/* Need a PGPROC to run CreateSharedMemoryAndSemaphores */
InitProcess();
which presents a problem, because StartBackgroundWorker() then does
/*
* If we're not supposed to have shared memory access, then detach from
* shared memory. If we didn't request shared memory access, the
* postmaster won't force a cluster-wide restart if we exit unexpectedly,
* so we'd better make sure that we don't mess anything up that would
* require that sort of cleanup.
*/
if ((worker->bgw_flags & BGWORKER_SHMEM_ACCESS) == 0)
{
ShutdownLatchSupport();
dsm_detach_all();
PGSharedMemoryDetach();
}
which presents a problem: We've initialized all kind of references to shared
memory, own a PGPROC, but have detached from shared memory.
In practice this will lead pretty quickly to a segfault, because process exit
will run proc_exit callbacks, which in turn will try to do a ProcKill(). Or
logging dereferences MyProc, or ...
It seems the above code block would need to at least do shmem_exit() before
the PGSharedMemoryDetach()?
This code has been introduced in
commit 4d155d8b08fe08c1a1649fdbad61c6dcf4a8671f
Author: Robert Haas <rhaas(at)postgresql(dot)org>
Date: 2014-05-07 14:54:43 -0400
Detach shared memory from bgworkers without shmem access.
Since the postmaster won't perform a crash-and-restart sequence
for background workers which don't request shared memory access,
we'd better make sure that they can't corrupt shared memory.
Patch by me, review by Tom Lane.
but before that things were just slightly differently broken...
I tested both the crash and that a shmem_exit() fixes it with an ugly hack in
regress.c. I don't really know how to write a good testcase for this, given
that the communication facilities of a unconnected bgworker are quite
limited... I guess the bgworker could create files or something :(.
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2021-08-02 07:01:42 | Re: EXEC_BACKEND vs bgworkers without BGWORKER_SHMEM_ACCESS |
Previous Message | houzj.fnst@fujitsu.com | 2021-08-02 06:30:21 | RE: Parallel Inserts (WAS: [bug?] Missed parallel safety checks..) |