[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: fix invalid use of siglongjmp due to threads
|
From: |
Jose E. Marchesi |
|
Subject: |
Re: fix invalid use of siglongjmp due to threads |
|
Date: |
Sun, 28 Jun 2020 16:43:30 +0200 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) |
Hi Bruno.
Interesting problem.
OK for master.
Thanks!
poke is a multi-threaded program. I see in gdb 3 threads from libgc:
(gdb) info threads
Id Target Id Frame
* 1 Thread 0x7ffff7fc4700 (LWP 16429) "poke" 0x00007ffff727051d in read
() at ../sysdeps/unix/syscall-template.S:84
2 Thread 0x7ffff5ae2700 (LWP 16433) "poke"
pthread_cond_wait@@GLIBC_2.3.2 ()
at ../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
3 Thread 0x7ffff52e1700 (LWP 16434) "poke"
pthread_cond_wait@@GLIBC_2.3.2 ()
at ../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
4 Thread 0x7ffff4ae0700 (LWP 16435) "poke"
pthread_cond_wait@@GLIBC_2.3.2 ()
at ../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
5 Thread 0x7ffff42df700 (LWP 16436) "poke" 0x00007ffff6f935d3 in
select () at ../sysdeps/unix/syscall-template.S:84
(gdb) where
#0 0x00007ffff727051d in read () at ../sysdeps/unix/syscall-template.S:84
#1 0x00007ffff7948a2d in rl_getc () from
/lib/x86_64-linux-gnu/libreadline.so.6
#2 0x00000000004053db in poke_getc (stream=0x7ffff725a8e0
<_IO_2_1_stdin_>) at pk-repl.c:142
#3 0x00007ffff79492ad in rl_read_key () from
/lib/x86_64-linux-gnu/libreadline.so.6
#4 0x00007ffff7932dd2 in readline_internal_char () from
/lib/x86_64-linux-gnu/libreadline.so.6
#5 0x00007ffff7933545 in readline () from
/lib/x86_64-linux-gnu/libreadline.so.6
#6 0x0000000000405878 in pk_repl () at pk-repl.c:313
#7 0x0000000000404d0c in main (argc=2, argv=0x7fffffffd808) at poke.c:683
(gdb) thread 2
[Switching to thread 2 (Thread 0x7ffff5ae2700 (LWP 16433))]
#0 pthread_cond_wait@@GLIBC_2.3.2 () at
../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
185 ../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S: Datei oder
Verzeichnis nicht gefunden.
(gdb) where
#0 pthread_cond_wait@@GLIBC_2.3.2 () at
../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1 0x00007ffff7b87937 in GC_wait_marker () from
/usr/lib/x86_64-linux-gnu/libgc.so.1
#2 0x00007ffff7b7dc3a in GC_help_marker () from
/usr/lib/x86_64-linux-gnu/libgc.so.1
#3 0x00007ffff7b85ebc in GC_mark_thread () from
/usr/lib/x86_64-linux-gnu/libgc.so.1
#4 0x00007ffff72676ba in start_thread (arg=0x7ffff5ae2700) at
pthread_create.c:333
#5 0x00007ffff6f9d41d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:109
(gdb) thread 3
[Switching to thread 3 (Thread 0x7ffff52e1700 (LWP 16434))]
#0 pthread_cond_wait@@GLIBC_2.3.2 () at
../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
185 in ../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
(gdb) where
#0 pthread_cond_wait@@GLIBC_2.3.2 () at
../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1 0x00007ffff7b87937 in GC_wait_marker () from
/usr/lib/x86_64-linux-gnu/libgc.so.1
#2 0x00007ffff7b7dc3a in GC_help_marker () from
/usr/lib/x86_64-linux-gnu/libgc.so.1
#3 0x00007ffff7b85ebc in GC_mark_thread () from
/usr/lib/x86_64-linux-gnu/libgc.so.1
#4 0x00007ffff72676ba in start_thread (arg=0x7ffff52e1700) at
pthread_create.c:333
#5 0x00007ffff6f9d41d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:109
(gdb) thread 4
[Switching to thread 4 (Thread 0x7ffff4ae0700 (LWP 16435))]
#0 pthread_cond_wait@@GLIBC_2.3.2 () at
../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
185 in ../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S
(gdb) where
#0 pthread_cond_wait@@GLIBC_2.3.2 () at
../sysdeps/unix/sysv/linux/x86_64/pthread_cond_wait.S:185
#1 0x00007ffff7b87937 in GC_wait_marker () from
/usr/lib/x86_64-linux-gnu/libgc.so.1
#2 0x00007ffff7b7dc3a in GC_help_marker () from
/usr/lib/x86_64-linux-gnu/libgc.so.1
#3 0x00007ffff7b85ebc in GC_mark_thread () from
/usr/lib/x86_64-linux-gnu/libgc.so.1
#4 0x00007ffff72676ba in start_thread (arg=0x7ffff4ae0700) at
pthread_create.c:333
#5 0x00007ffff6f9d41d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:109
And even if we soon won't use libgc any more, other libraries can create
threads at their own discretion. (E.g. macOS libc does this.)
But in a multithreaded program, any of the threads might get to execute any
signal handler. (Since you don't control the creation of the other threads,
and don't even have a way to enumerate them, you can't call pthread_sigmask
on them.)
In such a situation, siglongjmp may cause two threads to executed with the
same
stack area. At moment the two stack pointers get close to each other, it's
time
for Guru meditation.
This proposed patch fixes it.
The marker _GL_ASYNC_SAFE is purely for documentation purposes. See
gnulib-common.m4 for its (important) meaning.
Bruno
>From 03c1cfdfdbee84ddd1c09588c9290742ce61c283 Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Sat, 27 Jun 2020 17:25:10 +0200
Subject: [PATCH] Fix invalid use of siglongjmp due to threads.
* poke/pk-repl.c: Include <pthread.h>.
(ctrlc_thread): New variable.
(poke_sigint_handler): Mark as _GL_ASYNC_SAFE (purely a comment). When
invoked
in a different thread, redirect the signal to the ctrlc_thread.
(pk_repl): Initialize ctrlc_thread.
---
poke/pk-repl.c | 44 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 36 insertions(+), 8 deletions(-)
diff --git a/poke/pk-repl.c b/poke/pk-repl.c
index 38a53a5..c6dce90 100644
--- a/poke/pk-repl.c
+++ b/poke/pk-repl.c
@@ -19,6 +19,7 @@
#include <config.h>
#include <signal.h>
+#include <pthread.h>
#include <unistd.h>
#include <setjmp.h>
#include <stdlib.h>
@@ -37,9 +38,12 @@
#include "pk-utils.h"
#include "pk-map.h"
+/* The thread that contains the non-local entry point for reentering
+ the REPL. */
+static pthread_t volatile ctrlc_thread;
/* The non-local entry point for reentering the REPL. */
static sigjmp_buf /*volatile*/ ctrlc_buf;
-/* When nonzero, ctrlc_buf contains a valid value. */
+/* When nonzero, ctrlc_thread and ctrlc_buf contain valid values. */
static int volatile ctrlc_buf_valid;
/* This function is called repeatedly by the readline library, when
@@ -204,17 +208,36 @@ banner (void)
}
-static void
+static _GL_ASYNC_SAFE void
poke_sigint_handler (int sig)
{
if (ctrlc_buf_valid)
{
/* Abort the current command, and return to the REPL. */
- rl_free_line_state ();
- rl_cleanup_after_signal ();
- rl_line_buffer[rl_point = rl_end = rl_mark = 0] = 0;
- printf ("\n");
- siglongjmp (ctrlc_buf, 1);
+ if (!pthread_equal (pthread_self (), ctrlc_thread))
+ {
+ /* We are not in the thread that established the jmp_buf for
+ returning to the REPL. Redirect the signal to that thread,
+ and decline responsibility for future deliveries of this
+ signal. */
+ sigset_t mask;
+ if (sigemptyset (&mask) == 0
+ && sigaddset (&mask, SIGINT) == 0)
+ pthread_sigmask (SIG_BLOCK, &mask, NULL);
+
+ pthread_kill (ctrlc_thread, SIGINT);
+ }
+ else
+ {
+ /* We are in the thread that established the jmp_buf for
returning
+ to the REPL. Put the readline library into a sane state, then
+ unwind the stack. */
+ rl_free_line_state ();
+ rl_cleanup_after_signal ();
+ rl_line_buffer[rl_point = rl_end = rl_mark = 0] = 0;
+ printf ("\n");
+ siglongjmp (ctrlc_buf, 1);
+ }
}
else
{
@@ -289,7 +312,11 @@ pk_repl (void)
{
banner ();
- /* Arrange for the current line to be cancelled on SIGINT. */
+ /* Arrange for the current line to be cancelled on SIGINT.
+ Since some library code is also interested in SIGINT
+ (GNU libtextstyle, via gnulib module fatal-signal), it is better
+ to install it once, rather than to install and uninstall it once
+ in each round of the REP loop below. */
ctrlc_buf_valid = 0;
struct sigaction sa;
sa.sa_handler = poke_sigint_handler;
@@ -319,6 +346,7 @@ pk_repl (void)
rl_filename_quote_characters = " ";
rl_filename_quoting_function = escape_metacharacters;
+ ctrlc_thread = pthread_self ();
sigsetjmp (ctrlc_buf, 1);
ctrlc_buf_valid = 1;