mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Avoid calling gettext() in signal handlers.

Browse Source 
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
This commit is contained in:
Tom Lane 2022-01-17 13:30:04 -05:00
parent f3f467b8f6
commit 2131c049d3
1 changed files with 16 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
src/fe_utils/cancel.c
View File

@ -42,6 +42,13 @@
*/ */
static PGcancel *volatile cancelConn = NULL; 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). * CancelRequested is set when we receive SIGINT (or local equivalent).
* There is no provision in this module for resetting it; but applications * 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))) if (PQcancel(cancelConn, errbuf, sizeof(errbuf)))
{ {
write_stderr(_("Cancel request sent\n")); write_stderr(cancel_sent_msg);
} }
else else
{ {
write_stderr(_("Could not send cancel request: ")); write_stderr(cancel_not_sent_msg);
write_stderr(errbuf); write_stderr(errbuf);
} }
} }
@ -179,6 +186,9 @@ void
setup_cancel_handler(void (*callback) (void)) setup_cancel_handler(void (*callback) (void))
{ {
cancel_callback = callback; cancel_callback = callback;
cancel_sent_msg = _("Cancel request sent\n");
cancel_not_sent_msg = _("Could not send cancel request: ");
pqsignal(SIGINT, handle_sigint); pqsignal(SIGINT, handle_sigint);
} }
@ -203,11 +213,11 @@ consoleHandler(DWORD dwCtrlType)
{ {
if (PQcancel(cancelConn, errbuf, sizeof(errbuf))) if (PQcancel(cancelConn, errbuf, sizeof(errbuf)))
{ {
write_stderr(_("Cancel request sent\n")); write_stderr(cancel_sent_msg);
} }
else else
{ {
write_stderr(_("Could not send cancel request: ")); write_stderr(cancel_not_sent_msg);
write_stderr(errbuf); write_stderr(errbuf);
} }
} }
@ -225,6 +235,8 @@ void
setup_cancel_handler(void (*callback) (void)) setup_cancel_handler(void (*callback) (void))
{ {
cancel_callback = callback; cancel_callback = callback;
cancel_sent_msg = _("Cancel request sent\n");
cancel_not_sent_msg = _("Could not send cancel request: ");
InitializeCriticalSection(&cancelConnLock); InitializeCriticalSection(&cancelConnLock);