Re: pgsql: Consistently test for in-use shared memory.

Hi,

On 2019-04-04 00:16:55 +0000, Noah Misch wrote:
> Consistently test for in-use shared memory.
>
> postmaster startup scrutinizes any shared memory segment recorded in
> postmaster.pid, exiting if that segment matches the current data
> directory and has an attached process. When the postmaster.pid file was
> missing, a starting postmaster used weaker checks. Change to use the
> same checks in both scenarios. This increases the chance of a startup
> failure, in lieu of data corruption, if the DBA does "kill -9 `head -n1
> postmaster.pid` && rm postmaster.pid && pg_ctl -w start". A postmaster
> will no longer recycle segments pertaining to other data directories.
> That's good for production, but it's bad for integration tests that
> crash a postmaster and immediately delete its data directory. Such a
> test now leaks a segment indefinitely. No "make check-world" test does
> that. win32_shmem.c already avoided all these problems. In 9.6 and
> later, enhance PostgresNode to facilitate testing. Back-patch to 9.4
> (all supported versions).
>
> Reviewed by Daniel Gustafsson and Kyotaro HORIGUCHI.
>
> Discussion: https://postgr.es/m/20130911033341.GD225735@tornado.leadboat.com

My compiler now nitpicks:

In file included from /home/andres/src/postgresql/src/include/postgres.h:47,
from pg_shmem.c:20:
pg_shmem.c: In function ‘PGSharedMemoryCreate’:
/home/andres/src/postgresql/src/include/utils/elog.h:122:5: warning: this statement may fall through [-Wimplicit-fallthrough=]
do { \
^
/home/andres/src/postgresql/src/include/utils/elog.h:142:2: note: in expansion of macro ‘ereport_domain’
ereport_domain(elevel, TEXTDOMAIN, rest)
^~~~~~~~~~~~~~
pg_shmem.c:668:5: note: in expansion of macro ‘ereport’
ereport(FATAL,
^~~~~~~
pg_shmem.c:675:4: note: here
case SHMSTATE_ENOENT:
^~~~
All of PostgreSQL successfully made. Ready to install.

I think the SHMSTATE_ATTACHED case simply should grow a (unreachable)
break. But I'd also strongly suggest adding one to SHMSTATE_UNATTACHED -
because that's actually reachable, and just seems like a trap for person
adding another case:.

Greetings,

Andres Freund