emersion / mrsh

A minimal POSIX shell
MIT License
487 stars 35 forks source link

Job status not updated on SIGSTOP/SIGCONT #174

Open caseman opened 3 years ago

caseman commented 3 years ago
build/mrsh -mb
$ sleep 100&
$ kill -STOP $(pidof sleep)
$ jobs
[1] + Running sleep 100 &
$ kill $(pidof sleep)
$ 
[1] + Done(33024) sleep 100 &

The problem seems to be here: https://github.com/emersion/mrsh/blob/280fdf5ec59d6e66541d6b90e93ad0a3866b58ee/shell/job.c#L228

This check doesn't make sense to me, I would think you would only want to check WUNTRACED for children, since checking it for the parent shell process seems useless, but maybe I'm missing something. Also in order to detect SIGCONT the option WCONTINUED would need to be used where available, something like:

#ifdef WCONTINUED
               options |= WUNTRACED | WCONTINUED;
#else
               options |= WUNTRACED;
#endif

Although maybe that check is too pedantic, I'm not sure if any major platforms lack support for WCONTINUED today.

Happy to make a PR but I feel like I don't understand the intention of the check unless priv->child is supposed to mean something different than I'm thinking, is it only supposed to be set for grandchildren?

caseman commented 3 years ago

What I have learned so far:

I'm assuming the intermediate job process is a simplification somehow, other shells like zsh don't use one. But regardless of that it doesn't seem useful to report the job status based on the status of that process, other than when it exits entirely perhaps. I'll keep exploring.