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
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 |