pytest-dev / pytest-dev/execnet

safe_terminate() leaves the abandoned termfunc thread running and reports success anyway

Open
#429 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
102
Forks
47
PR merge metrics
No merged PRs in 30d

Description

safe_terminate() abandons the termfunc thread it spawned and then reports success while it is still running.

Versions: execnet 2.1.2, CPython 3.12, Linux. Reached via pytest-xdist 3.8.0 (NodeManager.teardown_nodesGroup.terminate(EXIT_TIMEOUT=10)), but the defect is in safe_terminate itself.

The code

# src/execnet/multi.py:331
def safe_terminate(execmodel, timeout, list_of_paired_functions) -> None:
    workerpool = WorkerPool(execmodel)

    def termkill(termfunc, killfunc) -> None:
        termreply = workerpool.spawn(termfunc)
        try:
            termreply.get(timeout=timeout)
        except OSError:
            killfunc()

    replylist = []
    for termfunc, killfunc in list_of_paired_functions:
        reply = workerpool.spawn(termkill, termfunc, killfunc)
        replylist.append(reply)
    for reply in replylist:
        reply.get()
    workerpool.waitall(timeout=timeout)

Two problems:

  1. The abandoned termfunc thread is never cancelled. When termreply.get(timeout=timeout) raises OSError, termkill calls killfunc() and returns — but the thread running termfunc is still blocked and keeps running. For Group.terminate() that termfunc is join_wait, i.e. gw.join(); gw._io.wait(), so it sits in subprocess.Popen.wait(). killfunc (gw._io.kill()) is issued, but nothing waits for it to take effect and nothing joins the thread.

  2. waitall's timeout result is discarded. WorkerPool.waitall() returns bool (gateway_base.py:470, return my_waitall_event.wait(timeout=timeout)). Line 349 ignores it, so safe_terminate returns normally — and therefore Group.terminate() returns normally — even when the abandoned threads from (1) are demonstrably still running.

Relatedly, Group.terminate(timeout) has no overall deadline: reply.get() on line 348 has no timeout at all, so the call can block indefinitely if a killfunc does.

Reproducer

"""safe_terminate() reports success while the termfunc thread it spawned is still running."""
import sys, threading, time
import execnet
from execnet.gateway_base import get_execmodel
from execnet.multi import safe_terminate

print("execnet", execnet.__version__, "| python", sys.version.split()[0])

execmodel = get_execmodel("thread")
entered, release = threading.Event(), threading.Event()
finished = []

def termfunc():                 # stands in for join_wait: gw.join(); gw._io.wait()
    entered.set()
    release.wait(60)
    finished.append(True)

def killfunc():                 # stands in for kill: gw._io.kill()
    pass                        # a kill that does not immediately reap the child

t0 = time.time()
safe_terminate(execmodel, 1.0, [(termfunc, killfunc)])
elapsed = time.time() - t0

print(f"safe_terminate(timeout=1.0) returned after {elapsed:.1f}s")
print(f"  termfunc entered : {entered.is_set()}")
print(f"  termfunc finished: {bool(finished)}   <-- still running, never cancelled")
release.set()
execnet 2.1.2 | python 3.12.13
safe_terminate(timeout=1.0) returned after 2.0s
  termfunc entered : True
  termfunc finished: False   <-- still running, never cancelled

killfunc is a no-op here to stand in for a kill that does not reap the child promptly; with a real popen gateway the same state is reached whenever Popen.wait() has not returned by the time waitall's timeout expires.

Why it matters

These threads are started by ThreadExecModel.start via _thread.start_new_thread (gateway_base.py:152), so they are not threading.Threads and threading._shutdown() never joins them. Once safe_terminate has returned, the caller is free to finish and the interpreter to finalize while those threads are still executing Python — they are then stopped at an arbitrary GIL checkpoint.

We hit this under pytest-xdist: teardown routinely exceeds xdist's 10s EXIT_TIMEOUT, so Group.terminate() takes the kill path on essentially every run, and faulthandler dumps taken in that window consistently show the leaked threads:

Thread ...:  File ".../execnet/multi.py", line 231 in join_wait      # x4, in subprocess.wait()
Thread ...:  File ".../execnet/multi.py", line 339 in termkill       # x3, in Reply.waitfinish
Thread ...:  File ".../execnet/multi.py", line 348 in safe_terminate # main, unbounded reply.get()
             File ".../execnet/multi.py", line 237 in terminate
             File ".../xdist/workermanage.py", line 117 in teardown_nodes

On one such run the process died with SIGSEGV in exactly this window, with the truncated dump ending inside Gateway.__repr__ (reached from kill() at multi.py:234) on a frame whose code object could no longer be resolved. I am not claiming execnet segfaults — a faulthandler.dump_traceback_later watchdog was walking those stacks concurrently and that race is a plausible cause on its own. But the leaked threads are what put live Python into that window in the first place.

Suggested directions

  • After killfunc(), wait on termreply again with a short grace period so the abandoned thread is actually observed to finish.
  • Propagate the waitall timeout instead of discarding it, so Group.terminate() can tell its caller that gateways did not come down.
  • Give Group.terminate(timeout) a single overall deadline rather than the current timeout at line 339 + unbounded wait at 348 + timeout at 349.

Happy to send a PR if you have a preference on the shape.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read safe_terminate and Group.terminate in src/execnet/multi.py, then inspect WorkerPool.waitall in gateway_base.py and the thread startup path in ThreadExecModel.start. Reproduce the timeout path first; done should mean abandoned termfunc work is accounted for, waitall's result is not silently discarded, and termination does not wait indefinitely.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
distributed-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.