Skip to content

Commit

Permalink
warn if an exception leaves the run-context
Browse files Browse the repository at this point in the history
This commit adds a warning that describes the possibility
of a stall, if an exception leaves the run-context.

The reason for the possible stall is that it is not clear
whether the code in the run-context did trigger a
subprocess exit. If it did not, and if no timeouts are
set, the exit handler of the run-context might wait
indefinitely for the subprocess to exit.

If the code ensures that all exceptions are caught, e.g.
by using a try-finally clause, and the subprocess exit
is triggered in case of an exception, the stalling
can be prevented.
  • Loading branch information
christian-monch committed Nov 5, 2023
1 parent 7443c8a commit 2b4c891
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
14 changes: 14 additions & 0 deletions datalad_next/runners/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"""
from __future__ import annotations

import logging
import subprocess
from collections.abc import Generator
from contextlib import contextmanager
Expand All @@ -23,6 +24,9 @@
)


lgr = logging.getLogger('datalad.ext.next.runners.run')


def _create_kill_wrapper(cls: type[Protocol]) -> type[Protocol]:
""" Extend ``cls`` to supports the "kill-interface"
Expand Down Expand Up @@ -252,6 +256,16 @@ def run(
else:
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.'
)
raise
finally:
# Arm the protocol, that will enable terminate signaling or kill
# signaling, if terminate_time or kill_time are not None.
Expand Down
19 changes: 19 additions & 0 deletions datalad_next/runners/tests/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,3 +378,22 @@ def test_run_nongenerator():
assert res['code'] == 0
assert res['stdout'] == 'outy'
assert res['stderr'] == 'error'


def test_run_exception_in_context(monkeypatch):
# Check that an exception in the context is logged and re-raises:
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,
):
raise ValueError('Something')
assert warnings[0].startswith('Possible stall:')

0 comments on commit 2b4c891

Please sign in to comment.