On Wed, 20 Jan 2021 at 05:23, Tom Lane <t...@sss.pgh.pa.us> wrote: > > Recursion is scary, but it should (I think) not be possible if this > is driven off CHECK_FOR_INTERRUPTS. There will certainly be none > of those in libbacktrace. >
We can also hold interrupts for the call, and it might be wise to do so. One point here is that it might be a good idea to suppress elog.c's > calls to functions in the error context stack. As we saw in another > connection recently, allowing that to happen makes for a *very* > large increase in the footprint of code that you are expecting to > work at any random CHECK_FOR_INTERRUPTS call site. > I strongly agree. Treat it as errhidecontext(). BTW, it also looks like the patch is doing nothing to prevent the > backtrace from being sent to the connected client. I'm not sure > what I think about whether it'd be okay from a security standpoint > to do that on the connection that requested the trace, but I sure > as heck don't want it to happen on connections that didn't. I don't see a good reason to send a bt to a client. Even though these backtraces won't be analysing debuginfo and populating args, locals, etc, it should still just go to the server log. > Maybe, given all of these things, we should forget using elog at > all and just emit the trace with fprintf(stderr). That causes quite a lot of pain with MemoryContextStats() already as it's frequently difficult to actually locate the output given the variations that exist in customer logging configurations. Sometimes stderr goes to a separate file or to journald. It's also much harder to locate the desired output since there's no log_line_prefix. I have a WIP patch floating around somewhere that tries to teach MemoryContextStats to write to the ereport channel when not called during an actual out-of-memory situation for that reason; an early version is somewhere in the archives. This is one of those "ok in development, painful in production" situations. So I'm not a big fan of pushing it to stderr, though I'd rather have that than not have the ability at all.