Skip to content

Commit 92e6c1c

Browse files
committed
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
1 parent 8b10746 commit 92e6c1c

File tree

1 file changed

+23
-4
lines changed

1 file changed

+23
-4
lines changed

src/bin/scripts/common.c

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,19 @@
2222
#include "fe_utils/string_utils.h"
2323

2424

25+
/*
26+
* Write a simple string to stderr --- must be safe in a signal handler.
27+
* We ignore the write() result since there's not much we could do about it.
28+
* Certain compilers make that harder than it ought to be.
29+
*/
30+
#define write_stderr(str) \
31+
do { \
32+
const char *str_ = (str); \
33+
int rc_; \
34+
rc_ = write(fileno(stderr), str_, strlen(str_)); \
35+
(void) rc_; \
36+
} while (0)
37+
2538
#define PQmblenBounded(s, e) strnlen(s, PQmblen(s, e))
2639

2740
static PGcancel *volatile cancelConn = NULL;
@@ -489,10 +502,13 @@ handle_sigint(SIGNAL_ARGS)
489502
if (PQcancel(cancelConn, errbuf, sizeof(errbuf)))
490503
{
491504
CancelRequested = true;
492-
fprintf(stderr, _("Cancel request sent\n"));
505+
write_stderr("Cancel request sent\n");
493506
}
494507
else
495-
fprintf(stderr, _("Could not send cancel request: %s"), errbuf);
508+
{
509+
write_stderr("Could not send cancel request: ");
510+
write_stderr(errbuf);
511+
}
496512
}
497513
else
498514
CancelRequested = true;
@@ -526,11 +542,14 @@ consoleHandler(DWORD dwCtrlType)
526542
{
527543
if (PQcancel(cancelConn, errbuf, sizeof(errbuf)))
528544
{
529-
fprintf(stderr, _("Cancel request sent\n"));
530545
CancelRequested = true;
546+
write_stderr("Cancel request sent\n");
531547
}
532548
else
533-
fprintf(stderr, _("Could not send cancel request: %s"), errbuf);
549+
{
550+
write_stderr("Could not send cancel request: ");
551+
write_stderr(errbuf);
552+
}
534553
}
535554
else
536555
CancelRequested = true;

0 commit comments

Comments
 (0)