Re: Prevent an error on attaching/creating a DSM/DSA from an interrupt handler.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Daniel Gustafsson <daniel(at)yesql(dot)se>, Rahila Syed <rahilasyed90(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Tomas Vondra <tomas(at)vondra(dot)me>
Subject: Re: Prevent an error on attaching/creating a DSM/DSA from an interrupt handler.
Date: 2025-05-20 02:29:25
Message-ID: 594293.1747708165@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> What I'm concerned about is that I think that (as I said on the other
> thread) is that ProcessGetMemoryContextInterrupt is not really at all
> safe to execute at an arbitrary CHECK_FOR_INTERRUPTS().

I agree.

> In my mind, the possible fixes here are (1) revert that patch, (2)
> redesign things so that the problematic code can only get called when
> we know that the backend state is sane, or (3) redesign the code so
> that it has fewer requirements for correct operation.

I want to argue for reverting, at least for v18. I do not think that
ProcessGetMemoryContextInterrupt is anywhere near release-quality.
I found out while poking into Valgrind leak reports that it leaks
memory --- and does so in TopMemoryContext. This is especially
unfortunate for something that's supposed to be used to investigate
memory consumption: a tool that affects the results under
consideration is not a great tool. The way I'd build it is to make
a special-purpose context that is a top-level context in its own
right (ie not a child of TopMemoryContext) and do all the work
therein, then reset or delete that context on the way out.

I also think it's remarkably poor design to have shoved this code
into mcxt.c, which is a very low-level module that has no business
having the #include dependencies that it acquired in 042a66291.
We've put some other stuff in mcxt.c that probably shouldn't be
there either, but bloating the file by 40% is a bit much.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Euler Taveira 2025-05-20 02:40:40 Re: Document default values for pgoutput options + fix missing initialization for "origin"
Previous Message David Rowley 2025-05-20 02:03:37 Re: Add comment explaining why queryid is int64 in pg_stat_statements