search

mitnk / cicada

An old-school bash-like Unix shell written in Rust
https://hugo.wang/cicada/
MIT License
981 stars 50 forks source link

shell fails to reap child processes in a timely manner #33

Closed godmar closed 3 years ago

godmar commented 3 years ago

If you run 

sleep 5 &
sleep 200

The first command should be reaped after 5 seconds, not after 200.

Similarly, if you run

sleep 1 &

this job isn't reaped until the user hits Enter again.

mitnk commented 3 years ago

Good catch! Just tried the same in bash, no such issue. (my first thought was bash do the same thing :)

Seems I need to add a signal handler for SIGCHLD.

Thanks for trying cicada.

godmar commented 3 years ago

You should call waitpid(-1,...) and then handle whatever job the process belongs to while you wait for a foreground job.

In addition, you do need a SIGCHLD handler, but this is tricky in Rust since the runtime is not async-signal -safe. Since the shell is either waiting for user input or for a foreground job you could unblock SIGCHLD right before reading user input, then block it right after.

mitnk commented 3 years ago

I tried a way to address this issue in #38, But feel it's not that ideal (but cannot find a better way yet): I have to sync some information between the signal hander and the main thread. One issue is if the same PID is used by another new process and it also got reaped by hander. Incorrect status may be fetched by main thread.

@godmar would you like to have a look and provide me any suggestions? Thanks a lot!