diff mbox series

[1/2] shell: Fix timeout process termination for zsh

Message ID 20210519063109.224352-1-lkml@jv-coder.de
State Rejected
Headers show
Series [1/2] shell: Fix timeout process termination for zsh | expand

Commit Message

Joerg Vehlow May 19, 2021, 6:31 a.m. UTC
From: Joerg Vehlow <joerg.vehlow@aox-tech.de>

NOTE: This fix should be part of the release,
because it fixes something broken in this release only!

On zsh an exit in the trap for SIGTERM is ignored,
but wait returns with TERM exit status (143).
This can only improve the situation for other processe,
e.g. if the wait is killed by the SIGTERM from the main process,
it will still terminate the timeout process now.

Fixes: a30410f6ad77 ("shell: Prevent orphan timeout sleep processes")

Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
---
 testcases/lib/tst_test.sh | 1 +
 1 file changed, 1 insertion(+)

Comments

Li Wang May 19, 2021, 10:18 a.m. UTC | #1
On Wed, May 19, 2021 at 2:31 PM Joerg Vehlow <lkml@jv-coder.de> wrote:
>
> From: Joerg Vehlow <joerg.vehlow@aox-tech.de>
>
> NOTE: This fix should be part of the release,
> because it fixes something broken in this release only!
>
> On zsh an exit in the trap for SIGTERM is ignored,

I'm wondering if this is zsh feature or a bug on a specific version?
if the latter, we probably have no need to fix it in LTP.
Btw, which zsh version do you use?

Odd, I tried on my laptop(Fedora34) with zsh-5.5.1-6.el8_1.2.x86_64,
but could NOT reproduce it.

my reproducer:
-----------------

# cat test.sh

echo "pid is $$" # send TERM to pid in another terminal

sleep 100 &
sleep_pid=$!

trap "kill $sleep_pid; exit;" TERM

wait $sleep_pid
[ $? -eq 143 ] && echo "FAIL"



> but wait returns with TERM exit status (143).
> This can only improve the situation for other processe,
> e.g. if the wait is killed by the SIGTERM from the main process,
> it will still terminate the timeout process now.
>
> Fixes: a30410f6ad77 ("shell: Prevent orphan timeout sleep processes")
>
> Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
> ---
>  testcases/lib/tst_test.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index 3a5651c01..1b25f4c44 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -472,6 +472,7 @@ _tst_timeout_process()
>         sleep_pid=$!
>         trap "kill $sleep_pid; exit" TERM
>         wait $sleep_pid
> +       [ $? -eq 143 ] && exit
>         trap - TERM
>         _tst_kill_test
>  }
> --
> 2.25.1
>


--
Regards,
Li Wang
Joerg Vehlow May 19, 2021, 10:26 a.m. UTC | #2
Hi Li,


On 5/19/2021 12:18 PM, Li Wang wrote:
>
> I'm wondering if this is zsh feature or a bug on a specific version?
> if the latter, we probably have no need to fix it in LTP.
> Btw, which zsh version do you use?
>
> Odd, I tried on my laptop(Fedora34) with zsh-5.5.1-6.el8_1.2.x86_64,
> but could NOT reproduce it.
>
> my reproducer:
> -----------------
>
> # cat test.sh
>
> echo "pid is $$" # send TERM to pid in another terminal
>
> sleep 100 &
> sleep_pid=$!
>
> trap "kill $sleep_pid; exit;" TERM
>
> wait $sleep_pid
> [ $? -eq 143 ] && echo "FAIL"
>

I may have something to do with subshells again...
I just tweaked your reproducer, to be a bit more like our "real 
scenario" and can reproduce it:

timeout()
{
     sleep 100 &
     sleep_pid=$!
     trap "echo Received TERM; kill $sleep_pid; exit;" TERM
     wait $sleep_pid
     [ $? -eq 143 ] && echo "FAIL"
}

timeout &
pid=$!
sleep 1
kill $pid


$ bash test.sh
Received TERM

$ zsh test.sh
Received TERM
FAIL

Jörg
Li Wang May 19, 2021, 10:29 a.m. UTC | #3
> I may have something to do with subshells again...
> I just tweaked your reproducer, to be a bit more like our "real
> scenario" and can reproduce it:

Yes, I can reproduce it now.

>
> timeout()
> {
>      sleep 100 &
>      sleep_pid=$!
>      trap "echo Received TERM; kill $sleep_pid; exit;" TERM
>      wait $sleep_pid
>      [ $? -eq 143 ] && echo "FAIL"
> }
>
> timeout &
> pid=$!
> sleep 1
> kill $pid
>
>
> $ bash test.sh
> Received TERM
>
> $ zsh test.sh
> Received TERM
> FAIL
>
> Jörg
>
>
diff mbox series

Patch

diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index 3a5651c01..1b25f4c44 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -472,6 +472,7 @@  _tst_timeout_process()
 	sleep_pid=$!
 	trap "kill $sleep_pid; exit" TERM
 	wait $sleep_pid
+	[ $? -eq 143 ] && exit
 	trap - TERM
 	_tst_kill_test
 }