pytest-dev / pytest-dev/execnet

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

Abierto
#429 1 comentario 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Lenguaje dominante
Python
Estrellas
102
Forks
47
Métricas de merge de PR
Sin PR fusionados en 30 d

Descripción

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.

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

Lee safe_terminate y Group.terminate en src/execnet/multi.py; después inspecciona WorkerPool.waitall en gateway_base.py y la ruta de inicio del hilo en ThreadExecModel.start. Reproduce primero la ruta de timeout; done debe significar que el trabajo de termfunc abandonado se contabiliza, que el resultado de waitall no se descarta silenciosamente y que la terminación no espera indefinidamente.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
python
Área
distributed-systems
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Activo
Claridad
Bastante claro
Aptitud para principiantes
45/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.