Re: on_exit_reset fails to clear DSM-related exit actions

On Mon, Mar 10, 2014 at 11:48 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Fri, Mar 7, 2014 at 2:40 PM, Robert Haas <robertmhaas@gmail.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.

I looked at this a little more.  It seems dsm_backend_shutdown()
already does almost the right thing; it fires all the on-detach
callbacks and unmaps the corresponding segments.  It does NOT unmap
the control segment, though, and processes that are unmapping the main
shared memory segment for safety should really be getting rid of the
control segment too (even though the consequences of corrupting the
control segment are much less bad).

One option is to just change that function to also unmap the control
segment, and maybe rename it to dsm_detach_all(), and then use that
everywhere.  The problem is that I'm not sure we really want to incur
the overhead of an extra munmap() during every backend exit, just to
get rid of a control segment which was going to be unmapped anyway by
process termination.  For that matter, I'm not sure why we bother
arranging that for the main shared memory segment, either: surely
whatever function the shmdt() and munmap() calls in IpcMemoryDetach
may have will be equally well-served by the forthcoming exit()?

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