* bug#70144: system* affects signal handlers @ 2024-04-02 14:22 Christopher Baines 2024-04-03 8:28 ` Mikael Djurfeldt 0 siblings, 1 reply; 5+ messages in thread From: Christopher Baines @ 2024-04-02 14:22 UTC (permalink / raw) To: 70144 [-- Attachment #1: Type: text/plain, Size: 544 bytes --] I've encountered a situation where signal handlers don't seem to run. With the following program, sending it SIGINT won't trigger the handler, however if you remove the system* call, then the handler will run. (use-modules (ice-9 threads)) (call-with-new-thread (lambda () ;; Remove the following system* call to fix the handler (system* "echo" "foo"))) (sigaction SIGINT (lambda (sig) (peek "SIGINT handler") (exit 1))) (for-each (lambda _ (sleep 1)) (iota 30)) (display "normal exit\n") [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 987 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#70144: system* affects signal handlers 2024-04-02 14:22 bug#70144: system* affects signal handlers Christopher Baines @ 2024-04-03 8:28 ` Mikael Djurfeldt 2024-05-02 14:17 ` Ludovic Courtès 0 siblings, 1 reply; 5+ messages in thread From: Mikael Djurfeldt @ 2024-04-03 8:28 UTC (permalink / raw) To: Christopher Baines; +Cc: Mikael Djurfeldt, 70144 [-- Attachment #1: Type: text/plain, Size: 1919 bytes --] system* temporarily re-binds signal handlers to prevent the child process from killing the parent. Thus, it is not thread safe with regard to SIGINT (or SIGQUIT if available). So, your code has a race condition with respect to the signal handler. This common resource can, in principle, be handled the usual way by, for example, utilizing a mutex: (use-modules (ice-9 threads)) (define sigint-mutex (make-mutex)) (define thread (call-with-new-thread (lambda () (with-mutex sigint-mutex (system* "echo" "foo"))))) (with-mutex sigint-mutex (sigaction SIGINT (lambda (sig) (peek "SIGINT handler") (exit 1))) (for-each (lambda _ (sleep 1)) (iota 30))) (join-thread thread) (display "normal exit\n") But if this was real code, another way would be to make sure that the code blocks are run in the order that you wish (which you did not want here since your very purpose was to provoke the collision of resources). I'm leaving this bug open. *Should* system* re-bind the signal handlers? Should it really protect itself from the child? If so, we should probably document this behaviour in the reference manual. Best regards, Mikael On Tue, Apr 2, 2024 at 4:28 PM Christopher Baines <mail@cbaines.net> wrote: > I've encountered a situation where signal handlers don't seem to > run. With the following program, sending it SIGINT won't trigger the > handler, however if you remove the system* call, then the handler will > run. > > (use-modules (ice-9 threads)) > > (call-with-new-thread > (lambda () > ;; Remove the following system* call to fix the handler > (system* "echo" "foo"))) > > (sigaction SIGINT > (lambda (sig) > (peek "SIGINT handler") > (exit 1))) > > (for-each > (lambda _ > (sleep 1)) > (iota 30)) > > (display "normal exit\n") > [-- Attachment #2: Type: text/html, Size: 2540 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#70144: system* affects signal handlers 2024-04-03 8:28 ` Mikael Djurfeldt @ 2024-05-02 14:17 ` Ludovic Courtès 2024-05-05 16:41 ` Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language 0 siblings, 1 reply; 5+ messages in thread From: Ludovic Courtès @ 2024-05-02 14:17 UTC (permalink / raw) To: Mikael Djurfeldt; +Cc: 70144, Josselin Poiret, Christopher Baines Hi, Mikael Djurfeldt <mikael@djurfeldt.com> skribis: > system* temporarily re-binds signal handlers to prevent the child process > from killing the parent. Thus, it is not thread safe with regard to SIGINT > (or SIGQUIT if available). So, your code has a race condition with respect > to the signal handler. This common resource can, in principle, be handled > the usual way by, for example, utilizing a mutex: [...] > I'm leaving this bug open. *Should* system* re-bind the signal handlers? > Should it really protect itself from the child? If so, we should probably > document this behaviour in the reference manual. Unless I’m mistaken, we can remove the ‘scm_dynwind_sigaction’ calls from ‘scm_system_star’: now that we use ‘posix_spawn’, this is all taken care of. This can be seen by running: strace -o /tmp/log.strace -f guile -c '(system* "/bin/sh" "-c" "echo foo $$")' … which shows pre-fork signal blocking (this is ‘internal_signal_block_all’ in spawni.c in glibc) followed, in the child (PID 28592 here), by a long series of ‘sigaction’ calls to reset handlers to their default behavior: --8<---------------cut here---------------start------------->8--- 28586 rt_sigprocmask(SIG_BLOCK, ~[], [], 8) = 0 28586 clone3({flags=CLONE_VM|CLONE_VFORK, exit_signal=SIGCHLD, stack=0x7f73b39b2000, stack_size=0x9000}, 88 <unfinished ...> 28592 rt_sigprocmask(SIG_BLOCK, NULL, ~[KILL STOP], 8) = 0 28592 rt_sigaction(SIGHUP, NULL, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0 28592 rt_sigaction(SIGHUP, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7f73b432d2a0}, NULL, 8) = 0 28592 rt_sigaction(SIGINT, NULL, {sa_handler=SIG_IGN, sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7f73b432d2a0}, 8) = 0 28592 rt_sigaction(SIGQUIT, NULL, {sa_handler=SIG_IGN, sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7f73b432d2a0}, 8) = 0 28592 rt_sigaction(SIGILL, NULL, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0 28592 rt_sigaction(SIGILL, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7f73b432d2a0}, NULL, 8) = 0 28592 rt_sigaction(SIGTRAP, NULL, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0 28592 rt_sigaction(SIGTRAP, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7f73b432d2a0}, NULL, 8) = 0 --8<---------------cut here---------------end--------------->8--- Josselin, can you confirm? Thanks, Ludo’. ^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#70144: system* affects signal handlers 2024-05-02 14:17 ` Ludovic Courtès @ 2024-05-05 16:41 ` Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language 2024-05-06 10:00 ` Ludovic Courtès 0 siblings, 1 reply; 5+ messages in thread From: Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language @ 2024-05-05 16:41 UTC (permalink / raw) To: Ludovic Courtès, Mikael Djurfeldt; +Cc: 70144, Christopher Baines [-- Attachment #1: Type: text/plain, Size: 2066 bytes --] Hi Ludo, Ludovic Courtès <ludo@gnu.org> writes: > Unless I’m mistaken, we can remove the ‘scm_dynwind_sigaction’ calls > from ‘scm_system_star’: now that we use ‘posix_spawn’, this is all taken > care of. > > This can be seen by running: > > strace -o /tmp/log.strace -f guile -c '(system* "/bin/sh" "-c" "echo foo $$")' > > … which shows pre-fork signal blocking (this is > ‘internal_signal_block_all’ in spawni.c in glibc) followed, in the child > (PID 28592 here), by a long series of ‘sigaction’ calls to reset > handlers to their default behavior: > > --8<---------------cut here---------------start------------->8--- > 28586 rt_sigprocmask(SIG_BLOCK, ~[], [], 8) = 0 > 28586 clone3({flags=CLONE_VM|CLONE_VFORK, exit_signal=SIGCHLD, stack=0x7f73b39b2000, stack_size=0x9000}, 88 <unfinished ...> > 28592 rt_sigprocmask(SIG_BLOCK, NULL, ~[KILL STOP], 8) = 0 > 28592 rt_sigaction(SIGHUP, NULL, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0 > 28592 rt_sigaction(SIGHUP, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7f73b432d2a0}, NULL, 8) = 0 > 28592 rt_sigaction(SIGINT, NULL, {sa_handler=SIG_IGN, sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7f73b432d2a0}, 8) = 0 > 28592 rt_sigaction(SIGQUIT, NULL, {sa_handler=SIG_IGN, sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7f73b432d2a0}, 8) = 0 > 28592 rt_sigaction(SIGILL, NULL, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0 > 28592 rt_sigaction(SIGILL, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7f73b432d2a0}, NULL, 8) = 0 > 28592 rt_sigaction(SIGTRAP, NULL, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=0}, 8) = 0 > 28592 rt_sigaction(SIGTRAP, {sa_handler=SIG_DFL, sa_mask=[], sa_flags=SA_RESTORER, sa_restorer=0x7f73b432d2a0}, NULL, 8) = 0 > --8<---------------cut here---------------end--------------->8--- > > Josselin, can you confirm? Yes, I believe this is all taken care of by our use of posix_spawn (which was the point in the first place :) ). Best, -- Josselin Poiret [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 682 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* bug#70144: system* affects signal handlers 2024-05-05 16:41 ` Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language @ 2024-05-06 10:00 ` Ludovic Courtès 0 siblings, 0 replies; 5+ messages in thread From: Ludovic Courtès @ 2024-05-06 10:00 UTC (permalink / raw) To: Josselin Poiret; +Cc: Mikael Djurfeldt, 70144-done, Christopher Baines Hi, Josselin Poiret <dev@jpoiret.xyz> skribis: > Yes, I believe this is all taken care of by our use of posix_spawn > (which was the point in the first place :) ). Yup! I pushed the fix as 4ae33f76d6b33ea0bedfa36050d44c88d08c2823. Thanks, Ludo’. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-05-06 10:00 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-02 14:22 bug#70144: system* affects signal handlers Christopher Baines 2024-04-03 8:28 ` Mikael Djurfeldt 2024-05-02 14:17 ` Ludovic Courtès 2024-05-05 16:41 ` Josselin Poiret via Bug reports for GUILE, GNU's Ubiquitous Extension Language 2024-05-06 10:00 ` Ludovic Courtès
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).