Re: Reducing relcache memory usage: deduping index shapes

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

In response to

Responses

Browse pgsql-hackers by date

  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