* fatal-signal: make multithread-safe
@ 2020-06-27 21:57 Bruno Haible
2020-06-27 22:42 ` Bruno Haible
0 siblings, 1 reply; 3+ messages in thread
From: Bruno Haible @ 2020-06-27 21:57 UTC (permalink / raw)
To: bug-gnulib
2020-06-27 Bruno Haible <bruno@clisp.org>
fatal-signal: Make multithread-safe.
* lib/fatal-signal.c: Include glthread/lock.h.
(at_fatal_signal_lock): New variable.
(at_fatal_signal): Use it.
(fatal_signals_block_lock, fatal_signals_block_counter): New variables.
(block_fatal_signals, unblock_fatal_signals): Use them.
* modules/fatal-signal (Depends-on): Add lock.
diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
index c8ff338..975393b 100644
--- a/lib/fatal-signal.c
+++ b/lib/fatal-signal.c
@@ -26,6 +26,7 @@
#include <signal.h>
#include <unistd.h>
+#include "glthread/lock.h"
#include "sig-handler.h"
#include "xalloc.h"
@@ -200,11 +201,16 @@ install_handlers (void)
}
+/* Lock that makes at_fatal_signal multi-thread safe. */
+gl_lock_define_initialized (static, at_fatal_signal_lock)
+
/* Register a cleanup function to be executed when a catchable fatal signal
occurs. */
void
at_fatal_signal (action_t action)
{
+ gl_lock_lock (at_fatal_signal_lock);
+
static bool cleanup_initialized = false;
if (!cleanup_initialized)
{
@@ -242,6 +248,8 @@ at_fatal_signal (action_t action)
actions[actions_count]. */
actions[actions_count].action = action;
actions_count++;
+
+ gl_lock_unlock (at_fatal_signal_lock);
}
@@ -269,20 +277,43 @@ init_fatal_signal_set (void)
}
}
+/* Lock and counter that allow block_fatal_signals/unblock_fatal_signals pairs
+ to occur in different threads and even overlap in time. */
+gl_lock_define_initialized (static, fatal_signals_block_lock)
+static unsigned int fatal_signals_block_counter = 0;
+
/* Temporarily delay the catchable fatal signals. */
void
block_fatal_signals (void)
{
- init_fatal_signal_set ();
- sigprocmask (SIG_BLOCK, &fatal_signal_set, NULL);
+ gl_lock_lock (fatal_signals_block_lock);
+
+ if (fatal_signals_block_counter++ == 0)
+ {
+ init_fatal_signal_set ();
+ sigprocmask (SIG_BLOCK, &fatal_signal_set, NULL);
+ }
+
+ gl_lock_unlock (fatal_signals_block_lock);
}
/* Stop delaying the catchable fatal signals. */
void
unblock_fatal_signals (void)
{
- init_fatal_signal_set ();
- sigprocmask (SIG_UNBLOCK, &fatal_signal_set, NULL);
+ gl_lock_lock (fatal_signals_block_lock);
+
+ if (fatal_signals_block_counter == 0)
+ /* There are more calls to unblock_fatal_signals() than to
+ block_fatal_signals(). */
+ abort ();
+ if (--fatal_signals_block_counter == 0)
+ {
+ init_fatal_signal_set ();
+ sigprocmask (SIG_UNBLOCK, &fatal_signal_set, NULL);
+ }
+
+ gl_lock_unlock (fatal_signals_block_lock);
}
diff --git a/modules/fatal-signal b/modules/fatal-signal
index 6683c79..de53de7 100644
--- a/modules/fatal-signal
+++ b/modules/fatal-signal
@@ -12,6 +12,7 @@ xalloc
stdbool
unistd
sigaction
+lock
sigprocmask
raise
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: fatal-signal: make multithread-safe
2020-06-27 21:57 fatal-signal: make multithread-safe Bruno Haible
@ 2020-06-27 22:42 ` Bruno Haible
2020-07-04 10:45 ` Bruno Haible
0 siblings, 1 reply; 3+ messages in thread
From: Bruno Haible @ 2020-06-27 22:42 UTC (permalink / raw)
To: bug-gnulib
That was good, but there's still a multithreading issue, as the signal handler
may be executing in a different thread.
2020-06-27 Bruno Haible <bruno@clisp.org>
fatal-signal: Make multithread-safe.
* lib/fatal-signal.c (at_fatal_signal): Don't free the old actions array.
diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
index 975393b..c6f8dac 100644
--- a/lib/fatal-signal.c
+++ b/lib/fatal-signal.c
@@ -239,8 +239,15 @@ at_fatal_signal (action_t action)
actions = new_actions;
actions_allocated = new_actions_allocated;
/* Now we can free the old actions array. */
+ /* No, we can't do that. If fatal_signal_handler is running in a
+ different thread and has already fetched the actions pointer (getting
+ old_actions) but not yet accessed its n-th element, that thread may
+ crash when accessing an element of the already freed old_actions
+ array. */
+ #if 0
if (old_actions != static_actions)
free (old_actions);
+ #endif
}
/* The two uses of 'volatile' in the types above (and ISO C 99 section
5.1.2.3.(5)) ensure that we increment the actions_count only after
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: fatal-signal: make multithread-safe
2020-06-27 22:42 ` Bruno Haible
@ 2020-07-04 10:45 ` Bruno Haible
0 siblings, 0 replies; 3+ messages in thread
From: Bruno Haible @ 2020-07-04 10:45 UTC (permalink / raw)
To: bug-gnulib
Another multithread-safety issue was lurking in module 'fatal-signal'.
This patch fixes it.
2020-07-04 Bruno Haible <bruno@clisp.org>
fatal-signal: Make multithread-safe.
* lib/fatal-signal.c (init_fatal_signals): Add comment.
(do_init_fatal_signal_set): New function, extracted from
init_fatal_signal_set.
(fatal_signal_set_once): New variable.
(init_fatal_signal_set): Use gl_once.
diff --git a/lib/fatal-signal.c b/lib/fatal-signal.c
index c6f8dac..8b27cc6 100644
--- a/lib/fatal-signal.c
+++ b/lib/fatal-signal.c
@@ -86,6 +86,10 @@ static int fatal_signals[] =
static void
init_fatal_signals (void)
{
+ /* This function is multithread-safe even without synchronization, because
+ if two threads execute it simultaneously, the fatal_signals[] array will
+ not change any more after the first of the threads has completed this
+ function. */
static bool fatal_signals_initialized = false;
if (!fatal_signals_initialized)
{
@@ -266,22 +270,25 @@ at_fatal_signal (action_t action)
static sigset_t fatal_signal_set;
static void
-init_fatal_signal_set (void)
+do_init_fatal_signal_set (void)
{
- static bool fatal_signal_set_initialized = false;
- if (!fatal_signal_set_initialized)
- {
- size_t i;
+ size_t i;
- init_fatal_signals ();
+ init_fatal_signals ();
- sigemptyset (&fatal_signal_set);
- for (i = 0; i < num_fatal_signals; i++)
- if (fatal_signals[i] >= 0)
- sigaddset (&fatal_signal_set, fatal_signals[i]);
+ sigemptyset (&fatal_signal_set);
+ for (i = 0; i < num_fatal_signals; i++)
+ if (fatal_signals[i] >= 0)
+ sigaddset (&fatal_signal_set, fatal_signals[i]);
+}
- fatal_signal_set_initialized = true;
- }
+/* Ensure that do_init_fatal_signal_set is called once only. */
+gl_once_define(static, fatal_signal_set_once)
+
+static void
+init_fatal_signal_set (void)
+{
+ gl_once (fatal_signal_set_once, do_init_fatal_signal_set);
}
/* Lock and counter that allow block_fatal_signals/unblock_fatal_signals pairs
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-07-04 10:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-27 21:57 fatal-signal: make multithread-safe Bruno Haible
2020-06-27 22:42 ` Bruno Haible
2020-07-04 10:45 ` Bruno Haible
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).