Fix inconsistent out-of-memory error reporting in dsa.c.

Commit 16be2fd1 introduced the flag DSA_ALLOC_NO_OOM to control whether
the DSA allocator would raise an error or return InvalidDsaPointer on
failure to allocate.  One edge case was not handled correctly: if we
fail to allocate an internal "span" object for a large allocation, we
would always return InvalidDsaPointer regardless of the flag; a caller
not expecting that could then dereference a null pointer.

This is a plausible explanation for a one-off report of a segfault.

Remove a redundant pair of braces so that all three stanzas that handle
DSA_ALLOC_NO_OOM match in style, for visual consistency.

While fixing inconsistencies, if FreePageManagerGet() can't supply the
pages that our book-keeping says it should be able to supply, then we
should always report a FATAL error.  Previously we treated that as a
regular allocation failure in one code path, but as a FATAL condition
in another.

Back-patch to 10, where dsa.c landed.

Author: Thomas Munro
Reported-by: Jakub Glapa
Discussion: https://postgr.es/m/CAEepm=2oPqXxyWQ-1o60tpOLrwkw=VpgNXqqF1VN2EyO9zKGQw@mail.gmail.com

diff --git a/src/backend/utils/mmgr/dsa.c b/src/backend/utils/mmgr/dsa.c
index cb868cfea250c6db3bcb56f6d572e2e48bac1364..62e6b652af9ed8a4ac6c093484212e3130988f28 100644 (file)
--- a/src/backend/utils/mmgr/dsa.c
+++ b/src/backend/utils/mmgr/dsa.c
@@ -693,7 +693,16 @@ dsa_allocate_extended(dsa_area *area, size_t size, int flags)
                /* Obtain a span object. */
                span_pointer = alloc_object(area, DSA_SCLASS_BLOCK_OF_SPANS);
                if (!DsaPointerIsValid(span_pointer))
+               {
+                       /* Raise error unless asked not to. */
+                       if ((flags & DSA_ALLOC_NO_OOM) == 0)
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_OUT_OF_MEMORY),
+                                                errmsg("out of memory"),
+                                                errdetail("Failed on DSA request of size %zu.",
+                                                                  size)));
                        return InvalidDsaPointer;
+               }
 
                LWLockAcquire(DSA_AREA_LOCK(area), LW_EXCLUSIVE);
 
@@ -790,12 +799,10 @@ dsa_allocate_extended(dsa_area *area, size_t size, int flags)
        {
                /* Raise error unless asked not to. */
                if ((flags & DSA_ALLOC_NO_OOM) == 0)
-               {
                        ereport(ERROR,
                                        (errcode(ERRCODE_OUT_OF_MEMORY),
                                         errmsg("out of memory"),
                                         errdetail("Failed on DSA request of size %zu.", size)));
-               }
                return InvalidDsaPointer;
        }
 
@@ -1669,13 +1676,14 @@ ensure_active_superblock(dsa_area *area, dsa_area_pool *pool,
                        return false;
                }
        }
+       /*
+        * This shouldn't happen: get_best_segment() or make_new_segment()
+        * promised that we can successfully allocate npages.
+        */
        if (!FreePageManagerGet(segment_map->fpm, npages, &first_page))
-       {
-               LWLockRelease(DSA_AREA_LOCK(area));
-               if (size_class != DSA_SCLASS_BLOCK_OF_SPANS)
-                       dsa_free(area, span_pointer);
-               return false;
-       }
+               elog(FATAL,
+                        "dsa_allocate could not find %zu free pages for superblock",
+                        npages);
        LWLockRelease(DSA_AREA_LOCK(area));
 
        /* Compute the start of the superblock. */