Re: on_exit_reset fails to clear DSM-related exit actions

On Mon, Mar 10, 2014 at 11:48 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Fri, Mar 7, 2014 at 2:40 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>> Hmm. So the problematic sequence of events is where a postmaster
>>> child forks, and then exits without exec-ing, perhaps because e.g.
>>> exec fails?
>
>> I've attempted a fix for this case. The attached patch makes
>> test_shm_mq fork() a child process that calls on_exit_reset() and then
>> exits. Without the fix I just pushed, this causes the tests to fail;
>> with this fix, this does not cause the tests to fail.
>
>> I'm not entirely sure that this is exactly right, but I think it's an
>> improvement.
>
> This looks generally sane to me, but I wonder whether you should also
> teach PGSharedMemoryDetach() to physically detach from DSM segments
> not just the main shmem seg. Or maybe better, adjust most of the
> places that call on_exit_reset and then PGSharedMemoryDetach to also
> make a detach-from-DSM call.

Hmm, good catch. Maybe we should invent a new function that is
defined to mean "detach ALL shared memory segments".

A related point that's been bugging me for a while, and has just
struck me again, is that background workers for which
BGWORKER_SHMEM_ACCESS is not passed probably ought to be detaching
shared memory (and DSMs). Currently, and since Alvaro originally
added the facility, the death of a non-BGWORKER_SHMEM_ACCESS backend
is used in postmaster.c to decide whether to call HandleChildCrash().
But such workers could still clobber shared memory arbitrarily; they
haven't unmapped it. Oddly, the code in postmaster.c is only cares
about the flag when checking EXIT_STATUS_0()/EXIT_STATUS_1(), not when
checking ReleasePostmasterChildSlot()...

> It looks like both sysv_shmem.c and
> win32_shmem.c have internal callers of PGSharedMemoryDetach, which
> probably shouldn't affect DSM.
>
> You could argue that that's unnecessary on the grounds that the postmaster
> will never have any DSM segments attached, but I think it would be
> good coding practice anyway. People who copy-and-paste those bits of
> code into other places are not likely to think of adding DSM calls.

Well, actually the postmaster WILL have the control segment attached
(not nothing else).

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company