Change API of ShmemAlloc() so it throws error rather than returning NULL.

A majority of callers seem to have believed that this was the API spec
already, because they omitted any check for a NULL result, and hence
would crash on an out-of-shared-memory failure.  The original proposal
was to just add such error checks everywhere, but that does nothing to
prevent similar omissions in future.  Instead, let's make ShmemAlloc()
throw the error (so we can remove the caller-side checks that do exist),
and introduce a new function ShmemAllocNoError() that has the previous
behavior of returning NULL, for the small number of callers that need
that and are prepared to do the right thing.  This also lets us remove
the rather wishy-washy behavior of printing a WARNING for out-of-shmem,
which never made much sense: either the caller has a strategy for
dealing with that, or it doesn't.  It's not ShmemAlloc's business to
decide whether a warning is appropriate.

The v10 release notes will need to call this out as a significant
source-code change.  It's likely that it will be a bug fix for
extension callers too, but if not, they'll need to change to using
ShmemAllocNoError().

This is nominally a bug fix, but the odds that it's fixing any live
bug are actually rather small, because in general the requests
being made by the unchecked callers were already accounted for in
determining the overall shmem size, so really they ought not fail.
Between that and the possible impact on extensions, no back-patch.

Discussion: <24843.1472563085@sss.pgh.pa.us>

diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index 1efe0201a795a315e158d47bf41786d9d17c85ad..cc3af2d6156b61d8d708a7df125fe0aa6b2e46e5 100644 (file)
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -163,14 +163,31 @@ InitShmemAllocation(void)
 /*
  * ShmemAlloc -- allocate max-aligned chunk from shared memory
  *
- * Assumes ShmemLock and ShmemSegHdr are initialized.
+ * Throws error if request cannot be satisfied.
  *
- * Returns: real pointer to memory or NULL if we are out
- *             of space.  Has to return a real pointer in order
- *             to be compatible with malloc().
+ * Assumes ShmemLock and ShmemSegHdr are initialized.
  */
 void *
 ShmemAlloc(Size size)
+{
+       void       *newSpace;
+
+       newSpace = ShmemAllocNoError(size);
+       if (!newSpace)
+               ereport(ERROR,
+                               (errcode(ERRCODE_OUT_OF_MEMORY),
+                                errmsg("out of shared memory (%zu bytes requested)",
+                                               size)));
+       return newSpace;
+}
+
+/*
+ * ShmemAllocNoError -- allocate max-aligned chunk from shared memory
+ *
+ * As ShmemAlloc, but returns NULL if out of space, rather than erroring.
+ */
+void *
+ShmemAllocNoError(Size size)
 {
        Size            newStart;
        Size            newFree;
@@ -206,11 +223,7 @@ ShmemAlloc(Size size)
 
        SpinLockRelease(ShmemLock);
 
-       if (!newSpace)
-               ereport(WARNING,
-                               (errcode(ERRCODE_OUT_OF_MEMORY),
-                                errmsg("out of shared memory")));
-
+       /* note this assert is okay with newSpace == NULL */
        Assert(newSpace == (void *) CACHELINEALIGN(newSpace));
 
        return newSpace;
@@ -293,7 +306,7 @@ ShmemInitHash(const char *name, /* table string name for shmem index */
         * The shared memory allocator must be specified too.
         */
        infoP->dsize = infoP->max_dsize = hash_select_dirsize(max_size);
-       infoP->alloc = ShmemAlloc;
+       infoP->alloc = ShmemAllocNoError;
        hash_flags |= HASH_SHARED_MEM | HASH_ALLOC | HASH_DIRSIZE;
 
        /* look it up in the shmem index */
@@ -364,12 +377,6 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
                         */
                        Assert(shmemseghdr->index == NULL);
                        structPtr = ShmemAlloc(size);
-                       if (structPtr == NULL)
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_OUT_OF_MEMORY),
-                                                errmsg("not enough shared memory for data structure"
-                                                               " \"%s\" (%zu bytes requested)",
-                                                               name, size)));
                        shmemseghdr->index = structPtr;
                        *foundPtr = FALSE;
                }
@@ -410,7 +417,7 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr)
        else
        {
                /* It isn't in the table yet. allocate and initialize it */
-               structPtr = ShmemAlloc(size);
+               structPtr = ShmemAllocNoError(size);
                if (structPtr == NULL)
                {
                        /* out of memory; remove the failed ShmemIndex entry */

diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 7cdb35541bf5e620335470c6dd873086b76035e6..4064b2033ccb89c02d0e52a6683557bd0bba82d9 100644 (file)
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -1184,12 +1184,6 @@ InitPredicateLocks(void)
                requestSize = mul_size((Size) max_table_size,
                                                           PredXactListElementDataSize);
                PredXact->element = ShmemAlloc(requestSize);
-               if (PredXact->element == NULL)
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_OUT_OF_MEMORY),
-                        errmsg("not enough shared memory for elements of data structure"
-                                       " \"%s\" (%zu bytes requested)",
-                                       "PredXactList", requestSize)));
                /* Add all elements to available list, clean. */
                memset(PredXact->element, 0, requestSize);
                for (i = 0; i < max_table_size; i++)
@@ -1255,12 +1249,6 @@ InitPredicateLocks(void)
                requestSize = mul_size((Size) max_table_size,
                                                           RWConflictDataSize);
                RWConflictPool->element = ShmemAlloc(requestSize);
-               if (RWConflictPool->element == NULL)
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_OUT_OF_MEMORY),
-                        errmsg("not enough shared memory for elements of data structure"
-                                       " \"%s\" (%zu bytes requested)",
-                                       "RWConflictPool", requestSize)));
                /* Add all elements to available list, clean. */
                memset(RWConflictPool->element, 0, requestSize);
                for (i = 0; i < max_table_size; i++)

diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 9a758bd91600b0839afadf3e3907e06a0902f8d5..33e7023656a6d6d675149061369870a7dc69efec 100644 (file)
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -194,14 +194,10 @@ InitProcGlobal(void)
         * between groups.
         */
        procs = (PGPROC *) ShmemAlloc(TotalProcs * sizeof(PGPROC));
+       MemSet(procs, 0, TotalProcs * sizeof(PGPROC));
        ProcGlobal->allProcs = procs;
        /* XXX allProcCount isn't really all of them; it excludes prepared xacts */
        ProcGlobal->allProcCount = MaxBackends + NUM_AUXILIARY_PROCS;
-       if (!procs)
-               ereport(FATAL,
-                               (errcode(ERRCODE_OUT_OF_MEMORY),
-                                errmsg("out of shared memory")));
-       MemSet(procs, 0, TotalProcs * sizeof(PGPROC));
 
        /*
         * Also allocate a separate array of PGXACT structures.  This is separate

diff --git a/src/include/storage/shmem.h b/src/include/storage/shmem.h
index 6468e6627ab601e19a23b056fb1f2fcc128deb40..2560e6c6da5764c3a118ba278568b0b6022d27da 100644 (file)
--- a/src/include/storage/shmem.h
+++ b/src/include/storage/shmem.h
@@ -35,6 +35,7 @@ typedef struct SHM_QUEUE
 extern void InitShmemAccess(void *seghdr);
 extern void InitShmemAllocation(void);
 extern void *ShmemAlloc(Size size);
+extern void *ShmemAllocNoError(Size size);
 extern bool ShmemAddrIsValid(const void *addr);
 extern void InitShmemIndex(void);
 extern HTAB *ShmemInitHash(const char *name, long init_size, long max_size,