| From: | David Rowley <dgrowleyml(at)gmail(dot)com> |
|---|---|
| To: | Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com> |
| Cc: | Yilin Zhang <jiezhilove(at)126(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, David Geier <geidav(dot)pg(at)gmail(dot)com> |
| Subject: | Re: Reducing relcache memory usage: deduping index shapes |
| Date: | 2026-10-14 23:00:57 |
| Message-ID: | CAApHDvqDcG_tNeqgmiET2N4m1qt6oX6Gv8T8viVmKpjaX5JFug@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, 14 Oct 2026 at 20:27, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> * Fixed bug in SystemContextRealloc() which would leak the old block
> on realloc() failure. The dlist_delete_from_thoroughly() can't be
> moved to after the realloc() as the dlist_member_check() can't access
> the old block. The best I could come up with is just doing
> dlist_push_tail() on the old block on failed realloc(). aset.c
> doesn't have this issue as it doesn't use dlist and adjusts the linked
> list pointers manually.
I'm a little disheartened here as I see you've gone and done exactly
what I said not to do. Maybe you didn't read the above text properly,
or maybe you didn't understand the issue I was talking about. If it's
the latter, then let me explain in more detail:
If you build with ILIST_DEBUG, dlist_delete_from_thoroughly() uses
dlist_member_check() to verify that the given node is actually in the
given list. This requires walking the linked list in a forward
direction, and to walk a linked list, we must access the "next"
pointer, which is stored inside the node's memory itself. Since this
memory has likely just been free'd by realloc(), it's not valid to
walk the list at this point. Effectively, dlist_member_check()'s
cur->next could segfault.
If you think I'm wrong about this, then please explain here so we can review.
If you didn't like my change, then an alternative way to work around
the accessing of the free'd memory would be to:
- dlist_delete_from_thoroughly(&ctx->allocations, &new_block->node);
+ dlist_delete_thoroughly(&new_block->node);
It's slightly less robust as we lose the validation that the node is
in the list.
David
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zsolt Parragi | 2026-10-14 23:02:59 | Re: pgstat: allow a stats kind to use its own dedicated dsa/dshash |
| Previous Message | Michael Paquier | 2026-10-14 22:55:34 | Re: pgstat: allow a stats kind to use its own dedicated dsa/dshash |