stats collector causes shared-memory-block leakage - Mailing list pgsql-hackers

While investigating Scott Goodwin's recent report of trouble on Mac OS
X, I have realized that we have an unpleasant little misbehavior in our
last several releases.  After a backend crash, the postmaster will
attempt to recycle (delete and recreate) the old shared memory segment.
However, if the stats collector is enabled, the two stats collection
subprocesses are still attached to the old segment.  Which means it
doesn't go away.  Instead the postmaster will set up shop with a new
segment.

This is not so bad in a system with large SHMMAX ... but if the SHMMAX
setting is too tight to permit a second shared memory segment to be
created, the postmaster fails.

The attached patch fixes the problem by causing the stats collector to
detach from shared memory, which it isn't using anyway.  Unless I hear
objections I will apply it to 7.4 and HEAD ... and I'm seriously
thinking of applying it to the 7.3 branch as well.
        regards, tom lane


*** src/backend/port/sysv_shmem.c.orig    Mon Oct 27 13:58:00 2003
--- src/backend/port/sysv_shmem.c    Thu Nov  6 18:10:02 2003
***************
*** 253,258 ****
--- 253,261 ----         return hdr;     } 
+     /* Make sure PGSharedMemoryAttach doesn't fail without need */
+     UsedShmemSegAddr = NULL;
+      /* Loop till we find a free IPC key */     NextShmemSegID = port * 1000; 
***************
*** 326,341 ****     hdr->totalsize = size;     hdr->freeoffset = MAXALIGN(sizeof(PGShmemHeader)); 
! 
!     if (ExecBackend && UsedShmemSegAddr == NULL && !makePrivate)
!     {
!         UsedShmemSegAddr = memAddress;
!         UsedShmemSegID = NextShmemSegID;
!     }      return hdr; }   /*  * Attach to shared memory and make sure it has a Postgres header
--- 329,360 ----     hdr->totalsize = size;     hdr->freeoffset = MAXALIGN(sizeof(PGShmemHeader)); 
!     /* Save info for possible future use */
!     UsedShmemSegAddr = memAddress;
!     UsedShmemSegID = NextShmemSegID;      return hdr; } 
+ /*
+  * PGSharedMemoryDetach
+  *
+  * Detach from the shared memory segment, if still attached.  This is not
+  * intended for use by the process that originally created the segment
+  * (it will have an on_shmem_exit callback registered to do that).  Rather,
+  * this is for subprocesses that have inherited an attachment and want to
+  * get rid of it.
+  */
+ void
+ PGSharedMemoryDetach(void)
+ {
+     if (UsedShmemSegAddr != NULL)
+     {
+         if (shmdt(UsedShmemSegAddr) < 0)
+             elog(LOG, "shmdt(%p) failed: %m", UsedShmemSegAddr);
+         UsedShmemSegAddr = NULL;
+     }
+ }  /*  * Attach to shared memory and make sure it has a Postgres header
*** src/backend/postmaster/pgstat.c.orig    Thu Sep 25 10:23:09 2003
--- src/backend/postmaster/pgstat.c    Thu Nov  6 18:10:46 2003
***************
*** 44,49 ****
--- 44,50 ---- #include "utils/memutils.h" #include "storage/backendid.h" #include "storage/ipc.h"
+ #include "storage/pg_shmem.h" #include "utils/rel.h" #include "utils/hsearch.h" #include "utils/ps_status.h"
***************
*** 399,404 ****
--- 400,408 ----      /* Close the postmaster's sockets, except for pgstat link */     ClosePostmasterPorts(false);
+ 
+     /* Drop our connection to postmaster's shared memory, as well */
+     PGSharedMemoryDetach();      pgstat_main(); 
*** src/include/storage/pg_shmem.h.orig    Sun Aug  3 23:01:43 2003
--- src/include/storage/pg_shmem.h    Thu Nov  6 18:09:50 2003
***************
*** 44,48 ****
--- 44,49 ---- extern PGShmemHeader *PGSharedMemoryCreate(uint32 size, bool makePrivate,                      int
port);extern bool PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2);
 
+ extern void PGSharedMemoryDetach(void);  #endif   /* PG_SHMEM_H */