Fix inconsistent out-of-memory error reporting in dsa.c.
authorThomas Munro <[email protected]>
Sun, 24 Feb 2019 21:54:12 +0000 (10:54 +1300)
committerThomas Munro <[email protected]>
Sun, 24 Feb 2019 22:11:40 +0000 (11:11 +1300)
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- 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

src/backend/utils/mmgr/dsa.c

index cb868cfea250c6db3bcb56f6d572e2e48bac1364..62e6b652af9ed8a4ac6c093484212e3130988f28 100644 (file)
@@ -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. */