On Tue, Apr 22, 2025 at 4:12 AM Daniel Gustafsson <dgustafs...@postgresql.org> wrote: > Do you mean that an interrupt handler will make DSA allocations? I don't > think > that would be something we'd want to allow regardless of this patch. Or did I > misunderstand the above?
My primary concern about the patch is that ProcessGetMemoryContextInterrupt() can be called from any CHECK_FOR_INTERRUPTS() and calls lots of DSA functions, including dsa_create() and, via PublishMemoryContext(), dsa_allocate0(). I'm shocked to hear that you and Andres think that's safe to do at any current or future CHECK_FOR_INTERRUPTS() anywhere in the code; but Andres seems very confident that it's fine, so perhaps I should just stop worrying and be happy that we have the feature. I thought that the issues here would be similar to https://commitfest.postgresql.org/patch/5473/ and https://commitfest.postgresql.org/patch/5330/ where it seems we need to go to fairly extraordinary lengths to try to make the operation safe -- the proposal there is essentially to have a CFI handler run around the executor state tree and replace every ExecProcNode pointer with a pointer to some new wrapper function which in turn does the actual query-plan printing. Even that requires some tricky footwork to get right, but the core idea is that you can't imagine that CHECK_FOR_INTERRUPTS() is a safe place to do arbitrary stuff, so you have to use it to trigger the actual work at some later point where it will be safe to do the stuff that you want to do. I suppose the difference in this case is that memory-allocation is a lower-level subsystem than query execution and therefore there are presumably fewer places where it's not safe to assume that you can do memory-allocation type operations. But I at least feel like DSA is a pretty high-level system that I wouldn't want to count on being able to access from a fairly-arbitrary point in the code, which is why I'm quite astonished to hear Andres basically saying "don't worry, it's all fine!". But my astonishment does not mean that he is wrong. -- Robert Haas EDB: http://www.enterprisedb.com
