Avoid trying to release a List's initial allocation via repalloc().
authorTom Lane <tgl@sss.pgh.pa.us>
Sun, 6 Oct 2019 16:06:30 +0000 (12:06 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sun, 6 Oct 2019 16:06:30 +0000 (12:06 -0400)
Commit 1cff1b95a included some code that supposed it could repalloc()
a memory chunk to a smaller size without risk of the chunk moving.
That was not a great idea, because it depended on undocumented behavior
of AllocSetRealloc, which commit c477f3e44 changed thereby breaking it.
(Not to mention that this code ought to work with other memory context
types, which might not work the same...)  So get rid of the repalloc
calls, and instead just wipe the now-unused ListCell array and/or tell
Valgrind it's NOACCESS, as if we'd freed it.

In cases where the initial list allocation had been quite large, this
could represent an annoying waste of space.  In principle we could
ameliorate that by allocating the initial cell array separately when
it exceeds some threshold.  But that would complicate new_list() which
is hot code, and the returns would materialize only in narrow cases.
On balance I don't think it'd be worth it.

Discussion: https://postgr.es/m/17059.1570208426@sss.pgh.pa.us

src/backend/nodes/list.c

index 6bf13ae0d4a24ef4a29b441c91b8611dfe28e0bf..5109ac9cde9dbfb24c070f829fed69f8678ae238 100644 (file)
@@ -18,6 +18,7 @@
 #include "postgres.h"
 
 #include "nodes/pg_list.h"
+#include "utils/memdebug.h"
 #include "utils/memutils.h"
 
 
@@ -172,8 +173,6 @@ enlarge_list(List *list, int min_size)
 
        if (list->elements == list->initial_elements)
        {
-               List       *newlist PG_USED_FOR_ASSERTS_ONLY;
-
                /*
                 * Replace original in-line allocation with a separate palloc block.
                 * Ensure it is in the same memory context as the List header.  (The
@@ -188,16 +187,18 @@ enlarge_list(List *list, int min_size)
                           list->length * sizeof(ListCell));
 
                /*
-                * Currently, asking aset.c to reduce the allocated size of the List
-                * header is pointless in terms of reclaiming space, unless the list
-                * is very long.  However, it seems worth doing anyway to cause the
-                * no-longer-needed initial_elements[] space to be cleared in
-                * debugging builds.
+                * We must not move the list header, so it's unsafe to try to reclaim
+                * the initial_elements[] space via repalloc.  In debugging builds,
+                * however, we can clear that space and/or mark it inaccessible.
+                * (wipe_mem includes VALGRIND_MAKE_MEM_NOACCESS.)
                 */
-               newlist = (List *) repalloc(list, offsetof(List, initial_elements));
-
-               /* That better not have failed, nor moved the list header */
-               Assert(newlist == list);
+#ifdef CLOBBER_FREED_MEMORY
+               wipe_mem(list->initial_elements,
+                                list->max_length * sizeof(ListCell));
+#else
+               VALGRIND_MAKE_MEM_NOACCESS(list->initial_elements,
+                                                                  list->max_length * sizeof(ListCell));
+#endif
        }
        else
        {
@@ -736,13 +737,16 @@ list_delete_nth_cell(List *list, int n)
                else
                {
                        /*
-                        * As in enlarge_list(), tell palloc code we're not using the
-                        * initial_elements space anymore.
+                        * As in enlarge_list(), clear the initial_elements[] space and/or
+                        * mark it inaccessible.
                         */
-                       List       *newlist PG_USED_FOR_ASSERTS_ONLY;
-
-                       newlist = (List *) repalloc(list, offsetof(List, initial_elements));
-                       Assert(newlist == list);
+#ifdef CLOBBER_FREED_MEMORY
+                       wipe_mem(list->initial_elements,
+                                        list->max_length * sizeof(ListCell));
+#else
+                       VALGRIND_MAKE_MEM_NOACCESS(list->initial_elements,
+                                                                          list->max_length * sizeof(ListCell));
+#endif
                }
                list->elements = newelems;
                list->max_length = newmaxlen;