Re: Printing backtrace of postgres processes

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, vignesh C <vignesh21@gmail.com>, torikoshia <torikoshia@oss.nttdata.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-02-09T08:13:44Z
Lists: pgsql-hackers
On Thu, Feb 08, 2024 at 12:25:18PM +0900, Michael Paquier wrote:
> In HandleLogBacktraceInterrupt(), we don't use backtrace_symbols() and
> rely on backtrace_symbols_fd() to avoid doing malloc() in the signal
> handler as mentioned in [1] back in 2022.  Perhaps the part about the
> fact that we don't use backtrace_symbols() should be mentioned
> explicitely in a comment rather than silently implied?  That's
> a very important point.

This has been itching me, so I have spent more time reading about
that, and while browsing signal(7) and signal-safety(7), I've first
noticed that this is not safe in the patch: 
+   write_stderr("logging current backtrace of process with PID %d:\n",
+                MyProcPid);

Note that there's a write_stderr_signal_safe().

Anyway, I've been digging around the signal-safety of backtrace(3)
(even looking a bit at some GCC code, brrr), and I am under the
impression that backtrace() is just by nature not safe and also
dangerous in signal handlers.  One example of issue I've found:
https://github.com/gperftools/gperftools/issues/838

This looks like enough ground to me to reject the patch.
--
Michael

Commits

  1. Perform apply of large transactions by parallel workers.

  2. Enhance pg_log_backend_memory_contexts() for auxiliary processes.

  3. Allow GRANT on pg_log_backend_memory_contexts().

  4. Move Perl test modules to a better namespace

  5. Unify PostgresNode's new() and get_new_node() methods

  6. Add backtrace support for error reporting