Re: approach to async-signal safety in bash

[Godmar Back]

On Wed, Jul 1, 2020 at 10:58 AM Chet Ramey <chet.ramey@case.edu> wrote:

>
> > Looking at the bash 5.0 code, I see some comments in the code about
> > strategies to protect the jobs array and other data structures from
> > arriving SIGCHLD signals, but I have questions about, for instance,
> these:
> >
> > - printable_job_status uses a 'static' variable "temp". However,
> > printable_job_status is called during the execution of the builtin
> command
> > "jobs" and here (I believe at least) without blocking or queuing SIGCHLD.
>
> I don't see how. It either calls list_{all,running,stopped}_jobs, which end
> up calling map_over_jobs and blocking SIGCHLD there, or jobs_builtin blocks
> SIGCHLD itself before calling list_one_job.
>
>
I think you are correct here. I set a breakpoint in there, but only checked
queue_sigchld
and not the actual SigBlk mask in /proc/*/status, which shows that SIGCHLD
was indeed blocked
on this path.

I did a little experiment, changing find_process to assert that SIGCHLD is
blocked (like the accompanying comment demands).
This fails on one of the provided unit tests for me, for instance:

[gback@oak bash-5.0]$ gdb ./bash
GNU gdb (GDB) Red Hat Enterprise Linux 8.2-11.el8
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html
>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./bash...done.
(gdb) run tests/jobs3.sub
Starting program: /home/staff/gback/cs3214/bash-5.0/bash tests/jobs3.sub
Missing separate debuginfos, use: yum debuginfo-install
glibc-2.28-101.el8.x86_64
warning: Loadable section ".note.gnu.property" outside of ELF segments
[Detaching after fork from child process 1976636]
[Detaching after fork from child process 1976637]
[Detaching after fork from child process 1976638]
[Detaching after fork from child process 1976640]
[Detaching after fork from child process 1976642]
[Detaching after fork from child process 1976643]
[Detaching after fork from child process 1976644]
[Detaching after fork from child process 1976645]
[Detaching after fork from child process 1976646]
[Detaching after fork from child process 1976647]
[Detaching after fork from child process 1976648]
[Detaching after fork from child process 1976651]
Waiting for job 0
job 0 returns 0
bash: jobs.c:1556: find_process: Assertion `_signal_is_blocked(SIGCHLD)'
failed.

Program received signal SIGABRT, Aborted.
0x00007ffff741570f in raise () from /lib64/libc.so.6
Missing separate debuginfos, use: yum debuginfo-install
ncurses-libs-6.1-7.20180224.el8.x86_64
(gdb) bt
#0  0x00007ffff741570f in raise () from /lib64/libc.so.6
#1  0x00007ffff73ffb25 in abort () from /lib64/libc.so.6
#2  0x00007ffff73ff9f9 in __assert_fail_base.cold.0 () from /lib64/libc.so.6
#3  0x00007ffff740dcc6 in __assert_fail () from /lib64/libc.so.6
#4  0x0000000000451c53 in find_process (pid=1976644, alive_only=1,
jobp=0x7fffffffd6a4) at jobs.c:1556
#5  0x0000000000455974 in waitchld (wpid=-1, block=0) at jobs.c:3681
#6  0x0000000000450f5d in cleanup_dead_jobs () at jobs.c:1046
#7  0x0000000000454909 in reap_dead_jobs () at jobs.c:3186
#8  0x000000000043d39f in execute_while_or_until (while_command=0x744130,
type=0) at execute_cmd.c:3612
#9  0x000000000043d2ee in execute_while_command (while_command=0x744130) at
execute_cmd.c:3579
#10 0x0000000000438bac in execute_command_internal (command=0x749e00,
asynchronous=0, pipe_in=-1, pipe_out=-1,
    fds_to_close=0x7360b0) at execute_cmd.c:955
#11 0x0000000000437f68 in execute_command (command=0x749e00) at
execute_cmd.c:394
#12 0x0000000000422d7f in reader_loop () at eval.c:175
#13 0x0000000000420a21 in main (argc=2, argv=0x7fffffffd9c8,
env=0x7fffffffd9e0) at shell.c:805
(gdb) p queue_sigchld
$1 = 0
(gdb)
[1]+  Stopped                 gdb ./bash
[gback@oak bash-5.0]$ ps f
    PID TTY      STAT   TIME COMMAND
1930944 pts/0    Ss     0:00 -bash
1976609 pts/0    Tl     0:00  \_ gdb ./bash
1976626 pts/0    t      0:00  |   \_ /home/staff/gback/cs3214/bash-5.0/bash
tests/jobs3.sub
1976645 pts/0    Z      0:00  |       \_ [sh] <defunct>
1976646 pts/0    Z      0:00  |       \_ [sh] <defunct>
1976647 pts/0    Z      0:00  |       \_ [sh] <defunct>
1976648 pts/0    Z      0:00  |       \_ [sh] <defunct>
1976651 pts/0    Z      0:00  |       \_ [sh] <defunct>
1976794 pts/0    R+     0:00  \_ ps f
[gback@oak bash-5.0]$ grep Sig /proc/1976626/status
SigQ: 1/1540473
SigPnd: 0000000000000000
SigBlk: 0000000000000000
SigIgn: 0000000000000004
SigCgt: 0000000000010000

> Therefore, if set -b is set, it could be reentered if a child process
> exits
> > at that time. This could clobber 'temp'.
> So this and the next one cover set -b, which by design has to notify
> asynchronously, out of the signal handler. Maybe it would be better to run
> it only if the shell is reading command input at the time the signal
> arrives.
>
>
Or use a signal-safe printf replacement that write()'s directly, but that
may become out of sync with stdout.