Skip to content

Commit

Permalink
[builtin/wait] Fix blocking after stopped process.
Browse files Browse the repository at this point in the history
BEFORE calling WaitForOne(), we should examine the shell state and see
if we're already there.

Addresses issue #1005.
  • Loading branch information
Andy C committed Feb 18, 2022
1 parent 27155a1 commit 8beba18
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 25 deletions.
38 changes: 18 additions & 20 deletions osh/builtin_process.py
Expand Up @@ -129,22 +129,23 @@ def _Run(self, cmd_val):
if arg.n:
# Loop until there is one fewer process running, there's nothing to wait
# for, or there's a signal
target = self.job_state.NumRunning() - 1
status = 0
while True:
result = self.waiter.WaitForOne()
if result == process.W1_OK:
status = self.waiter.last_status
elif result == process.W1_ECHILD:
# nothing to wait for, or interrupted
status = 127
break
elif result >= 0 and result != pyos.UNTRAPPED_SIGWINCH: # signal
status = 128 + result
break

if self.job_state.NumRunning() == target:
break
n = self.job_state.NumRunning()
if n == 0:
status = 127
else:
target = n - 1
status = 0
while self.job_state.NumRunning() > target:
result = self.waiter.WaitForOne()
if result == process.W1_OK:
status = self.waiter.last_status
elif result == process.W1_ECHILD:
# nothing to wait for, or interrupted
status = 127
break
elif result >= 0 and result != pyos.UNTRAPPED_SIGWINCH: # signal
status = 128 + result
break

return status

Expand All @@ -156,7 +157,7 @@ def _Run(self, cmd_val):
# But how to fix this?

status = 0
while True:
while self.job_state.NumRunning() != 0:
result = self.waiter.WaitForOne()
if result == process.W1_ECHILD:
# nothing to wait for, or interrupted. status is 0
Expand All @@ -165,9 +166,6 @@ def _Run(self, cmd_val):
status = 128 + result
break

if self.job_state.NumRunning() == 0:
break

return status

# Get list of jobs. Then we need to check if they are ALL stopped.
Expand Down
3 changes: 3 additions & 0 deletions osh/cmd_eval.py
Expand Up @@ -1361,6 +1361,9 @@ def _Execute(self, node):
Also runs trap handlers.
"""
# TODO: Do this in "leaf" nodes? SimpleCommand, DBracket, DParen should
# call self.DoTick()? That will RunPendingTraps and check the Ctrl-C flag,
# and maybe throw an exception.
self.RunPendingTraps()

# This has to go around redirect handling because the process sub could be
Expand Down
30 changes: 26 additions & 4 deletions spec/stateful/job_control.py
Expand Up @@ -95,7 +95,6 @@ def bug_721(sh):

time.sleep(0.1)

log('Ctrl-C')
ctrl_c(sh)
expect_prompt(sh)

Expand All @@ -108,7 +107,6 @@ def bug_721(sh):
sh.sendline('fg')
expect_no_job(sh)

log('End')
sh.sendline('')
expect_prompt(sh)

Expand All @@ -122,13 +120,37 @@ def bug_1005(sh):
sh.sendline('sleep 10')

time.sleep(0.1)
if 1: # TODO: remove hack for OSH
stop_process__hack('sleep')
else:
ctrl_z(sh)

ctrl_z(sh)
sh.expect(r'.*Stopped.*')

sh.sendline('wait')
sh.sendline('echo status=$?')
sh.expect(r"status=0")
sh.expect('status=0')


@register(skip_shells=['dash'])
def bug_1005_wait_n(sh):
'sleep 10 then Ctrl-Z then wait -n should not hang'

expect_prompt(sh)

sh.sendline('sleep 10')

time.sleep(0.1)
if 1: # TODO: remove hack for OSH
stop_process__hack('sleep')
else:
ctrl_z(sh)

sh.expect(r'.*Stopped.*')

sh.sendline('wait -n')
sh.sendline('echo status=$?')
sh.expect('status=127')


@register()
Expand Down
2 changes: 1 addition & 1 deletion test/stateful.sh
Expand Up @@ -37,7 +37,7 @@ run() {
readonly FIRST=''

signals-quick() {
spec/stateful/signals.py $FIRST --osh-failures-allowed 1 \
spec/stateful/signals.py $FIRST \
$OSH bash "$@"
}

Expand Down

0 comments on commit 8beba18

Please sign in to comment.