Skip to content

Commit

Permalink
improve stall-warning, add wait-debug message
Browse files Browse the repository at this point in the history
This change ensures that a stall-warning is only emitted,
if `kill_time` is not set.

In addition, the commit adds debug output, when the
run-context-manager enters the loop to wait for
process exit. This should help to debug "hanging"
conditions.
  • Loading branch information
christian-monch committed Nov 6, 2023
1 parent 2b4c891 commit 994ac9e
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 18 deletions.
19 changes: 12 additions & 7 deletions datalad_next/runners/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import logging
import subprocess
import sys
from collections.abc import Generator
from contextlib import contextmanager
from pathlib import Path
Expand Down Expand Up @@ -257,14 +258,14 @@ def run(
try:
yield KillingResultGenerator(result)
except:
import sys, time
e = sys.exc_info()[1]
lgr.warning(
f'Possible stall: exception ({e!r}) raised in '
f'run-context ({cmd!r}), waiting for subprocess exit. If the '
f'subprocess exit was not triggered yet, and if no termination-'
f' or kill-timeout is active, this might wait forever.'
)
if kill_time is None:
lgr.warning(
f'Possible stall: exception ({e!r}) raised in '
f'run-context ({cmd!r}) without kill-timeout. Waiting for '
f'subprocess exit, If subprocess exit was not triggered yet, '
f'this might wait forever.'
)
raise
finally:
# Arm the protocol, that will enable terminate signaling or kill
Expand All @@ -276,5 +277,9 @@ def run(
# the result code of the terminated process.
# NOTE: if terminate_time and kill_time are both None, this might
# loop forever.
lgr.debug(
f'Waiting for termination of {cmd!r}, terminate_time: '
f'{terminate_time!r}, kill_time: {kill_time!r}'
)
for _ in result:
pass
33 changes: 22 additions & 11 deletions datalad_next/runners/tests/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from pathlib import Path
from queue import Queue
from random import randint
from typing import Generator

import pytest

Expand Down Expand Up @@ -380,20 +379,32 @@ def test_run_nongenerator():
assert res['stderr'] == 'error'


def test_run_exception_in_context(monkeypatch):
# Check that an exception in the context is logged and re-raises:
@pytest.mark.parametrize(
'termination_time,kill_time,expect_warning',
[
(1, None, True),
(None, 1, False),
(1, 1, False),
(None, None, True),
]
)
def test_run_exception_in_context(monkeypatch, termination_time, kill_time, expect_warning):
# Check that an exception in the context is re-raised, and that a
# stall-warning is emitted, if no timeout was set.
warnings = []
monkeypatch.setattr(
'datalad_next.runners.run.lgr.warning',
lambda s: warnings.append(s)
)

with pytest.raises(ValueError):
with run([
sys.executable, '-u', '-c',
'import time\n'
'time.sleep(5)\n'
],
StdOutCaptureGeneratorProtocol,
):
with run([sys.executable, '-u', '-c', 'import time\ntime.sleep(2)\n'],
StdOutCaptureGeneratorProtocol,
terminate_time=termination_time,
kill_time=kill_time):
raise ValueError('Something')
assert warnings[0].startswith('Possible stall:')
if expect_warning:
assert len(warnings) == 1
assert warnings[0].startswith('Possible stall:')
else:
assert len(warnings) == 0

0 comments on commit 994ac9e

Please sign in to comment.