python / python/cpython

`subprocess.Popen.poll` race condition returns without polling the child

Aperta
#127,050 1 commento 1 reazione 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

stdlib type-bug
Lingua principale
Python
Stelle
77.2k
Fork
35.9k
Metriche di merge delle PR
Metriche PR in attesa

Descrizione

Bug report

Bug description:

While working on GH-127049, I noticed that on non-Windows platforms Popen.poll can return None even for an exited child when Popen.poll races against Popen.[poll | wait] in another thread.

I think Popen.poll is violating its docs when this happens, because it returns without checking the child.

Writing a reproducer for this is tricky, but the following hits the race condition more than half the time on my machine:

from __future__ import annotations

import os
import subprocess
from concurrent.futures import ThreadPoolExecutor

with ThreadPoolExecutor(1) as executor:
    process = subprocess.Popen(
        (
            "python",
            "-c",
            "import time; time.sleep(1)",
        ),
        stdin=subprocess.DEVNULL,
        stdout=subprocess.DEVNULL,
        stderr=subprocess.DEVNULL,
    )
    executor.submit(process.wait)

    try:
        os.waitid(os.P_PID, process.pid, os.WEXITED | os.WNOWAIT)
    except ChildProcessError:
        # P_PIDFD would avoid this ECHILD, but writing the reproducer this way for
        # portability.
        pass
    assert process.poll() is not None

The culprit is https://github.com/python/cpython/blob/v3.14.0a1/Lib/subprocess.py#L1989-L1992, from d65ba51e245ffdd155bc1e7b8884fc943048111f.

IIUC the reason that poll can't block on _waitpid_lock.acquire here is because poll must be non-blocking API, but a wait call can make a blocking call (waitpid without WNOHANG) while holding that lock.

IIUC this should be fixable with the two-step (1) wait-without-reaping and (2) hold a lock to atomically reap-without-waiting & set .returncode approach described at https://github.com/python/cpython/issues/82811#issuecomment-1093845693 and https://github.com/python/cpython/issues/86724#issuecomment-1093894017 and used by Trio. That approach should also be able to fix case 1 of the thread-unsafety ignored in GH-20010, but not case 2[^1]. It could be a bit of a pain though[^2].

To be clear about impact, though, I have only seen this poll retval bug happen while testing a fix for GH-127049. And in that situation, a very slight modification to said fix for GH-127049 can easily avoid this bug (as well as case 1 from GH-20010).

[^1]: Tangent: Case 2 should be fixable on Linux >= 5.4 (via pidfd_open & P_PIDFD). (It's possibly also fixable on BSDs & macOS, though I am not currently familiar there. What happens to an already-existing kevent for a PID after that PID has been freed? Does it still work, like a pidfd? It looks like Trio assumes that it does.)

[^2]: Does the "fail gracefully when SIGCLD is set to be ignored or waiting for child processes has otherwise been disabled for our process" case need different handling than the "an external caller reaped the PID and stole the return code from the Popen" case? From Popen's perspective (assuming a platform without pidfd/kqueue), both of these cases look like an ECHILD (either from "WNOWAIT waitid (wait-without-reaping) or WNOHANG wait[p]id (reap-without-waiting)). Is it okay if we can't disambiguate between these two cases?

CPython versions tested on:

3.9, 3.10, 3.11, 3.12, 3.13, 3.14, CPython main branch

Operating systems tested on:

Linux

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Direzione di ricerca

La issue rimanda alle righe 1989-1992 di Lib/subprocess.py; inizia da lì ed esegui su Linux il riproduttore fornito di ThreadPoolExecutor. Leggi le discussioni sul locking di wait/poll in GH-82811 e GH-86724, quindi verifica che poll non restituisca più None per un processo figlio terminato e rimanga non bloccante.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
python
Ambito
operating-systems
Tipo di issue
Bug
Difficoltà
5/5
Tempo stimato
Più di una settimana
Stato di attività
Tranquilla
Chiarezza
Abbastanza chiara
Idoneità per principianti
35/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.