Put in place some defenses against being fooled by accidental match of

shared memory segment ID.  If we can't access the existing shmem segment,
it must not be relevant to our data directory.  If we can access it,
then attach to it and check for an actual match to the data directory.
This should avoid some cases of failure-to-restart-after-boot without
introducing any significant risk of failing to detect a still-running
old backend.

diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c
index bff83b5621f9dcd5151f751529355f1afb54b028..3c3f928ca2d72da58586ff93013db5ce008b3aaf 100644 (file)
--- a/src/backend/port/sysv_shmem.c
+++ b/src/backend/port/sysv_shmem.c
@@ -10,7 +10,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/port/sysv_shmem.c,v 1.39 2004/10/13 01:25:11 neilc Exp $
+ *   $PostgreSQL: pgsql/src/backend/port/sysv_shmem.c,v 1.40 2004/11/09 21:30:13 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #ifdef HAVE_SYS_IPC_H
 #include 
 #endif
@@ -40,6 +41,12 @@ typedef int IpcMemoryId;     /* shared memory ID returned by shmget(2) */
 
 #define IPCProtection  (0600)  /* access/modify by user only */
 
+#ifdef SHM_SHARE_MMU           /* use intimate shared memory on Solaris */
+#define PG_SHMAT_FLAGS         SHM_SHARE_MMU
+#else
+#define PG_SHMAT_FLAGS         0
+#endif
+
 
 unsigned long UsedShmemSegID = 0;
 void      *UsedShmemSegAddr = NULL;
@@ -135,16 +142,10 @@ InternalIpcMemoryCreate(IpcMemoryKey memKey, uint32 size)
    on_shmem_exit(IpcMemoryDelete, Int32GetDatum(shmid));
 
    /* OK, should be able to attach to the segment */
-#ifdef SHM_SHARE_MMU
-   /* use intimate shared memory on Solaris */
-   memAddress = shmat(shmid, 0, SHM_SHARE_MMU);
-#else
-
 #ifdef EXEC_BACKEND
-   memAddress = shmat(shmid, UsedShmemSegAddr, 0);
+   memAddress = shmat(shmid, UsedShmemSegAddr, PG_SHMAT_FLAGS);
 #else
-   memAddress = shmat(shmid, NULL, 0);
-#endif
+   memAddress = shmat(shmid, NULL, PG_SHMAT_FLAGS);
 #endif
 
    if (memAddress == (void *) -1)
@@ -188,20 +189,26 @@ IpcMemoryDelete(int status, Datum shmId)
  * PGSharedMemoryIsInUse
  *
  * Is a previously-existing shmem segment still existing and in use?
+ *
+ * The point of this exercise is to detect the case where a prior postmaster
+ * crashed, but it left child backends that are still running.  Therefore
+ * we only care about shmem segments that are associated with the intended
+ * DataDir.  This is an important consideration since accidental matches of
+ * shmem segment IDs are reasonably common.
  */
 bool
 PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2)
 {
    IpcMemoryId shmId = (IpcMemoryId) id2;
    struct shmid_ds shmStat;
+#ifndef WIN32
+   struct stat statbuf;
+   PGShmemHeader *hdr;
+#endif
 
    /*
     * We detect whether a shared memory segment is in use by seeing
     * whether it (a) exists and (b) has any processes are attached to it.
-    *
-    * If we are unable to perform the stat operation for a reason other than
-    * nonexistence of the segment (most likely, because it doesn't belong
-    * to our userid), assume it is in use.
     */
    if (shmctl(shmId, IPC_STAT, &shmStat) < 0)
    {
@@ -212,13 +219,58 @@ PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2)
         */
        if (errno == EINVAL)
            return false;
-       /* Else assume segment is in use */
+       /*
+        * EACCES implies that the segment belongs to some other userid,
+        * which means it is not a Postgres shmem segment (or at least,
+        * not one that is relevant to our data directory).
+        */
+       if (errno == EACCES)
+           return false;
+       /*
+        * Otherwise, we had better assume that the segment is in use.
+        * The only likely case is EIDRM, which implies that the segment
+        * has been IPC_RMID'd but there are still processes attached to it.
+        */
        return true;
    }
-   /* If it has attached processes, it's in use */
-   if (shmStat.shm_nattch != 0)
-       return true;
-   return false;
+
+   /* If it has no attached processes, it's not in use */
+   if (shmStat.shm_nattch == 0)
+       return false;
+
+   /*
+    * Try to attach to the segment and see if it matches our data directory.
+    * This avoids shmid-conflict problems on machines that are running
+    * several postmasters under the same userid.  On Windows, which doesn't
+    * have useful inode numbers, we can't do this so we punt and assume there
+    * is a conflict.
+    */
+#ifndef WIN32
+   if (stat(DataDir, &statbuf) < 0)
+       return true;            /* if can't stat, be conservative */
+
+   hdr = (PGShmemHeader *) shmat(shmId, NULL, PG_SHMAT_FLAGS);
+
+   if (hdr == (PGShmemHeader *) -1)
+       return true;            /* if can't attach, be conservative */
+
+   if (hdr->magic != PGShmemMagic ||
+       hdr->device != statbuf.st_dev ||
+       hdr->inode != statbuf.st_ino)
+   {
+       /*
+        * It's either not a Postgres segment, or not one for my data
+        * directory.  In either case it poses no threat.
+        */
+       shmdt((void *) hdr);
+       return false;
+   }
+
+   /* Trouble --- looks a lot like there's still live backends */
+   shmdt((void *) hdr);
+#endif
+
+   return true;
 }
 
 
@@ -247,6 +299,9 @@ PGSharedMemoryCreate(uint32 size, bool makePrivate, int port)
    void       *memAddress;
    PGShmemHeader *hdr;
    IpcMemoryId shmid;
+#ifndef WIN32
+   struct stat statbuf;
+#endif
 
 #ifdef EXEC_BACKEND
    /* If Exec case, just attach and return the pointer */
@@ -345,6 +400,17 @@ PGSharedMemoryCreate(uint32 size, bool makePrivate, int port)
    hdr->creatorPID = getpid();
    hdr->magic = PGShmemMagic;
 
+#ifndef WIN32
+   /* Fill in the data directory ID info, too */
+   if (stat(DataDir, &statbuf) < 0)
+       ereport(FATAL,
+               (errcode_for_file_access(),
+                errmsg("could not stat data directory \"%s\": %m",
+                       DataDir)));
+   hdr->device = statbuf.st_dev;
+   hdr->inode = statbuf.st_ino;
+#endif
+
    /*
     * Initialize space allocation status for segment.
     */
@@ -397,15 +463,7 @@ PGSharedMemoryAttach(IpcMemoryKey key, IpcMemoryId *shmid)
    if ((*shmid = shmget(key, sizeof(PGShmemHeader), 0)) < 0)
        return NULL;
 
-   hdr = (PGShmemHeader *) shmat(*shmid,
-                                 UsedShmemSegAddr,
-#ifdef SHM_SHARE_MMU
-   /* use intimate shared memory on Solaris */
-                                 SHM_SHARE_MMU
-#else
-                                 0
-#endif
-       );
+   hdr = (PGShmemHeader *) shmat(*shmid, UsedShmemSegAddr, PG_SHMAT_FLAGS);
 
    if (hdr == (PGShmemHeader *) -1)
        return NULL;            /* failed: must be some other app's */

diff --git a/src/include/storage/pg_shmem.h b/src/include/storage/pg_shmem.h
index 126e977b9e3e9c328976c9cfedbf49d3a5a96fed..8b13ece9fe2c2d936bf241f008b3109937c4840e 100644 (file)
--- a/src/include/storage/pg_shmem.h
+++ b/src/include/storage/pg_shmem.h
@@ -17,7 +17,7 @@
  * Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/storage/pg_shmem.h,v 1.11 2004/08/29 04:13:10 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/storage/pg_shmem.h,v 1.12 2004/11/09 21:30:18 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -27,10 +27,14 @@
 typedef struct PGShmemHeader   /* standard header for all Postgres shmem */
 {
    int32       magic;          /* magic # to identify Postgres segments */
-#define PGShmemMagic  679834892
+#define PGShmemMagic  679834893
    pid_t       creatorPID;     /* PID of creating process */
    uint32      totalsize;      /* total size of segment */
    uint32      freeoffset;     /* offset to first free space */
+#ifndef WIN32                  /* Windows doesn't have useful inode#s */
+   dev_t       device;         /* device data directory is on */
+   ino_t       inode;          /* inode number of data directory */
+#endif
 } PGShmemHeader;