From: Tom Lane <tgl@sss.pgh.pa.us>
To: Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com>
cc: pgsql-bugs@lists.postgresql.org,
        Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Subject: Re: BUG #15804: Assertion failure when using logging_collector with
 EXEC_BACKEND
In-reply-to: <26332.1557863524@sss.pgh.pa.us>
References: <15804-3721117bf40fb654@postgresql.org>
 <27CD45B5-88F7-4A09-8B31-EFE41FC37FE1@crunchydata.com>
 <26332.1557863524@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Tue, 14 May 2019 15:52:04 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <29643.1557869457.1@sss.pgh.pa.us>
Date: Tue, 14 May 2019 17:30:57 -0400
Message-ID: <29644.1557869457@sss.pgh.pa.us>
Precedence: bulk

I wrote:
> It might be better to give up the assertion in PGSharedMemoryNoReAttach,
> and just make it work more like PGSharedMemoryDetach, ie "detach if
> UsedShmemSegAddr is set, else do nothing".  I don't remember for sure,
> but if we do that, there might be no functional difference anymore
> between those two functions, in which case we might as well merge 'em.

Apropos of this: I notice that commit 57431a911 is already relying
on PGSharedMemoryDetach to do the "right thing" here, because that
gets called in the non-EXEC_BACKEND code path, cf SysLogger_Start:

            /* Drop our connection to postmaster's shared memory, as well */
            dsm_detach_all();
            PGSharedMemoryDetach();

I'm also not too happy that this comment doesn't mention the fact that
we'd (now) only be attached to shmem in the case of a postmaster restart.

BTW, it looks to me like dsm_detach_all doesn't get called anywhere in
the EXEC_BACKEND case, which seems like a separate bug.

			regards, tom lane