Windows has support for some signals[0], like SIGTERM and SIGINT. SIGINT
must be handled with care on Windows since it is handled in a separate
thread. SIGTERM however can be handled in a similar way to UNIX-like
systems. I audited a few pqsignal calls that were blocked by WIN32 to
see if they could become used, and made some adjustments. Definitely
hoping for someone with more Windows knowledge to audit this.
In addition, I found that signal_cleanup() in pg_test_fsync.c was not
using signal-safe functions, so I went ahead and fixed those to use
their signal-safe equivalents.
[0]: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/signal
--
Tristan Partin
Neon (https://neon.tech)
From 2c4573afe55e83d3b5c99460c7429ff7cf3adcdf Mon Sep 17 00:00:00 2001
From: Tristan Partin <tris...@neon.tech>
Date: Thu, 6 Jul 2023 15:25:14 -0500
Subject: [PATCH v1 1/2] Use signal-safe functions in signal handler
According to signal-safety(7), exit(3) and puts(3) are not safe to call
in a signal handler.
---
src/bin/pg_test_fsync/pg_test_fsync.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/bin/pg_test_fsync/pg_test_fsync.c b/src/bin/pg_test_fsync/pg_test_fsync.c
index 435df8d808..2fc4b69444 100644
--- a/src/bin/pg_test_fsync/pg_test_fsync.c
+++ b/src/bin/pg_test_fsync/pg_test_fsync.c
@@ -596,8 +596,8 @@ signal_cleanup(SIGNAL_ARGS)
if (needs_unlink)
unlink(filename);
/* Finish incomplete line on stdout */
- puts("");
- exit(1);
+ write(STDOUT_FILENO, "", 1);
+ _exit(1);
}
#ifdef HAVE_FSYNC_WRITETHROUGH
--
Tristan Partin
Neon (https://neon.tech)
From ce4cbf8e720301e7ddbd2f0816c1f03c82138515 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tris...@neon.tech>
Date: Thu, 6 Jul 2023 15:17:36 -0500
Subject: [PATCH v1 2/2] Cleanup some signal usage on Windows
SIGTERM can be handled the same as on a UNIX-like system. SIGINT, on the
other hand, requires special considerations. The Windows documentation
says the following:
> SIGINT is not supported for any Win32 application. When a CTRL+C
> interrupt occurs, Win32 operating systems generate a new thread to
> specifically handle that interrupt. This can cause a single-thread
> application, such as one in UNIX, to become multithreaded and cause
> unexpected behavior.
Therefore, if all the SIGINT handler does is read from memory and/or
exit, we can use the same handler. If the signal handler writes to
memory, such as setting a boolean, the original thread would never
become aware of the changed state.
---
src/bin/initdb/initdb.c | 17 ++++-------------
src/bin/pg_basebackup/pg_receivewal.c | 5 +----
src/bin/pg_basebackup/pg_recvlogical.c | 9 ++++-----
src/bin/pg_test_fsync/pg_test_fsync.c | 3 ---
4 files changed, 9 insertions(+), 25 deletions(-)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 3f4167682a..cac58eae2a 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -2773,26 +2773,17 @@ void
setup_signals(void)
{
/* some of these are not valid on Windows */
-#ifdef SIGHUP
- pqsignal(SIGHUP, trapsig);
-#endif
-#ifdef SIGINT
+ pqsignal(SIGTERM, trapsig);
+
+#ifndef WIN32
pqsignal(SIGINT, trapsig);
-#endif
-#ifdef SIGQUIT
+ pqsignal(SIGHUP, trapsig);
pqsignal(SIGQUIT, trapsig);
-#endif
-#ifdef SIGTERM
- pqsignal(SIGTERM, trapsig);
-#endif
/* Ignore SIGPIPE when writing to backend, so we can clean up */
-#ifdef SIGPIPE
pqsignal(SIGPIPE, SIG_IGN);
-#endif
/* Prevent SIGSYS so we can probe for kernel calls that might not work */
-#ifdef SIGSYS
pqsignal(SIGSYS, SIG_IGN);
#endif
}
diff --git a/src/bin/pg_basebackup/pg_receivewal.c b/src/bin/pg_basebackup/pg_receivewal.c
index d0a4079d50..55143c4037 100644
--- a/src/bin/pg_basebackup/pg_receivewal.c
+++ b/src/bin/pg_basebackup/pg_receivewal.c
@@ -611,14 +611,11 @@ StreamLog(void)
* When SIGINT/SIGTERM are caught, just tell the system to exit at the next
* possible moment.
*/
-#ifndef WIN32
-
static void
sigexit_handler(SIGNAL_ARGS)
{
time_to_stop = true;
}
-#endif
int
main(int argc, char **argv)
@@ -838,9 +835,9 @@ main(int argc, char **argv)
* Trap signals. (Don't do this until after the initial password prompt,
* if one is needed, in GetConnection.)
*/
+ pqsignal(SIGTERM, sigexit_handler);
#ifndef WIN32
pqsignal(SIGINT, sigexit_handler);
- pqsignal(SIGTERM, sigexit_handler);
#endif
/*
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index f3c7937a1d..1228dc03db 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -644,10 +644,6 @@ error:
conn = NULL;
}
-/*
- * Unfortunately we can't do sensible signal handling on windows...
- */
-#ifndef WIN32
/*
* When SIGINT/SIGTERM are caught, just tell the system to exit at the next
@@ -659,6 +655,9 @@ sigexit_handler(SIGNAL_ARGS)
time_to_abort = true;
}
+/* SIGHUP is not defined on Windows */
+#ifndef WIN32
+
/*
* Trigger the output file to be reopened.
*/
@@ -921,9 +920,9 @@ main(int argc, char **argv)
* Trap signals. (Don't do this until after the initial password prompt,
* if one is needed, in GetConnection.)
*/
+ pqsignal(SIGTERM, sigexit_handler);
#ifndef WIN32
pqsignal(SIGINT, sigexit_handler);
- pqsignal(SIGTERM, sigexit_handler);
pqsignal(SIGHUP, sighup_handler);
#endif
diff --git a/src/bin/pg_test_fsync/pg_test_fsync.c b/src/bin/pg_test_fsync/pg_test_fsync.c
index 2fc4b69444..263f5867a2 100644
--- a/src/bin/pg_test_fsync/pg_test_fsync.c
+++ b/src/bin/pg_test_fsync/pg_test_fsync.c
@@ -109,9 +109,6 @@ main(int argc, char *argv[])
pqsignal(SIGTERM, signal_cleanup);
#ifndef WIN32
pqsignal(SIGALRM, process_alarm);
-#endif
-#ifdef SIGHUP
- /* Not defined on win32 */
pqsignal(SIGHUP, signal_cleanup);
#endif
--
Tristan Partin
Neon (https://neon.tech)
