diff --git a/problemtools/run/program.py b/problemtools/run/program.py index 38c789d8..5d095b5b 100644 --- a/problemtools/run/program.py +++ b/problemtools/run/program.py @@ -97,6 +97,11 @@ def __run_wait(argv, infile, outfile, errfile, timelim, memlim, working_director pid = os.fork() if pid == 0: # child try: + # Keep the submission and everything it forks in a process + # group that can be cleaned up after the submission exits. + # Unlike setsid(), this does not detach the submission from + # its parent session or terminal. + os.setpgrp() # The Python interpreter internally sets some signal dispositions # to SIG_IGN (notably SIGPIPE), and unless we reset them manually # this leaks through to the program we exec. That can has some @@ -132,7 +137,36 @@ def __run_wait(argv, infile, outfile, errfile, timelim, memlim, working_director # Unreachable log.error('Unreachable part of run_wait reached') os.kill(os.getpid(), signal.SIGTERM) - (pid, status, rusage) = os.wait4(pid, 0) + try: + # Keep the group leader unreaped while killing its descendants so + # a recycled pid cannot make killpg target an unrelated group. + (_, status, rusage) = os.wait4(pid, os.WNOWAIT) + except KeyboardInterrupt: + # Ctrl-C interrupts the parent, not the separate submission group. + # Clean up the whole group before re-raising so both serial and + # threaded verification can abort without leaking submissions. + try: + os.killpg(pid, signal.SIGKILL) + except ProcessLookupError: + pass + try: + os.wait4(pid, 0) + except ChildProcessError: + # macOS may already reap the child during WNOWAIT. + pass + raise + + # The process waited for above may have left descendants behind. + try: + os.killpg(pid, signal.SIGKILL) + except ProcessLookupError: + pass + try: + os.wait4(pid, 0) + except ChildProcessError: + # macOS exposes WNOWAIT but wait4() still reaps on that platform; + # the first wait already collected the status there. + pass return status, rusage.ru_utime + rusage.ru_stime @staticmethod diff --git a/tests/test_run_program.py b/tests/test_run_program.py new file mode 100644 index 00000000..59036a53 --- /dev/null +++ b/tests/test_run_program.py @@ -0,0 +1,71 @@ +import os +import resource +import signal +import textwrap +import time + +from problemtools.run.executable import Executable +from problemtools.run import program + + +def _process_exists(pid: int) -> bool: + try: + os.kill(pid, 0) + except ProcessLookupError: + return False + return True + + +def test_run_kills_forked_children_after_time_limit(tmp_path, monkeypatch): + # macOS rejects raising the child stack soft limit to its finite hard + # limit. Keep the CPU limit real; bypass only this unrelated platform + # limitation so the test exercises the process-group cleanup itself. + original_try_limit = program.limit.try_limit + + def try_limit(limit, soft, hard): + if limit == resource.RLIMIT_STACK: + return + return original_try_limit(limit, soft, hard) + + monkeypatch.setattr(program.limit, 'try_limit', try_limit) + child_pid_file = tmp_path / 'child.pid' + child_pid_tmp_file = tmp_path / 'child.pid.tmp' + program_path = tmp_path / 'forking-timeout.py' + program_path.write_text( + textwrap.dedent( + f"""\ + #!/usr/bin/env python3 + import os + import signal + import time + + child_pid = os.fork() + if child_pid == 0: + with open({str(child_pid_tmp_file)!r}, 'w') as child_pid_output: + child_pid_output.write(str(os.getpid())) + os.replace({str(child_pid_tmp_file)!r}, {str(child_pid_file)!r}) + while True: + time.sleep(60) + + while not os.path.exists({str(child_pid_file)!r}): + time.sleep(0.01) + os.kill(os.getpid(), signal.SIGXCPU) + """ + ) + ) + program_path.chmod(0o755) + + status, _ = Executable(str(program_path)).run(timelim=1, work_dir=str(tmp_path)) + + child_pid = int(child_pid_file.read_text()) + try: + assert os.WIFSIGNALED(status) + assert os.WTERMSIG(status) == signal.SIGXCPU + for _ in range(200): + if not _process_exists(child_pid): + break + time.sleep(0.01) + assert not _process_exists(child_pid) + finally: + if _process_exists(child_pid): + os.kill(child_pid, signal.SIGKILL)