From d18ec312f9f36c220aa1a1497d3173729c862d66 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 17 Jan 2022 13:30:04 -0500 Subject: [PATCH] Avoid calling gettext() in signal handlers. It seems highly unlikely that gettext() can be relied on to be async-signal-safe. psql used to understand that, but someone got it wrong long ago in the src/bin/scripts/ version of handle_sigint, and then the bad idea was perpetuated when those two versions were unified into src/fe_utils/cancel.c. I'm unsure why there have not been field complaints about this ... maybe gettext() is signal-safe once it's translated at least one message? But we have no business assuming any such thing. In cancel.c (v13 and up), I preserved our ability to localize "Cancel request sent" messages by invoking gettext() before the signal handler is set up. In earlier branches I just made src/bin/scripts/ not localize those messages, as psql did then. (Just for extra unsafety, the src/bin/scripts/ version was invoking fprintf() from a signal handler. Sigh.) Noted while fixing signal-safety issues in PQcancel() itself. Back-patch to all supported branches. Discussion: https://postgr.es/m/2937814.1641960929@sss.pgh.pa.us --- src/fe_utils/cancel.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/fe_utils/cancel.c b/src/fe_utils/cancel.c index 0a385647df..b5a04480f0 100644 --- a/src/fe_utils/cancel.c +++ b/src/fe_utils/cancel.c @@ -42,6 +42,13 @@ */ static PGcancel *volatile cancelConn = NULL; +/* + * Predetermined localized error strings --- needed to avoid trying + * to call gettext() from a signal handler. + */ +static const char *cancel_sent_msg = NULL; +static const char *cancel_not_sent_msg = NULL; + /* * CancelRequested is set when we receive SIGINT (or local equivalent). * There is no provision in this module for resetting it; but applications @@ -158,11 +165,11 @@ handle_sigint(SIGNAL_ARGS) { if (PQcancel(cancelConn, errbuf, sizeof(errbuf))) { - write_stderr(_("Cancel request sent\n")); + write_stderr(cancel_sent_msg); } else { - write_stderr(_("Could not send cancel request: ")); + write_stderr(cancel_not_sent_msg); write_stderr(errbuf); } } @@ -179,6 +186,9 @@ void setup_cancel_handler(void (*callback) (void)) { cancel_callback = callback; + cancel_sent_msg = _("Cancel request sent\n"); + cancel_not_sent_msg = _("Could not send cancel request: "); + pqsignal(SIGINT, handle_sigint); } @@ -203,11 +213,11 @@ consoleHandler(DWORD dwCtrlType) { if (PQcancel(cancelConn, errbuf, sizeof(errbuf))) { - write_stderr(_("Cancel request sent\n")); + write_stderr(cancel_sent_msg); } else { - write_stderr(_("Could not send cancel request: ")); + write_stderr(cancel_not_sent_msg); write_stderr(errbuf); } } @@ -225,6 +235,8 @@ void setup_cancel_handler(void (*callback) (void)) { cancel_callback = callback; + cancel_sent_msg = _("Cancel request sent\n"); + cancel_not_sent_msg = _("Could not send cancel request: "); InitializeCriticalSection(&cancelConnLock);