asyncio: _PidfdChildWatcher leaks the pidfd if waitpid() raises an unexpected error

[deadlovelll]

Bug report

Bug description:

_PidfdChildWatcher._do_wait() closes the pidfd after the try/except, but the except only catches ChildProcessError:

def _do_wait(self, pid, pidfd, callback, args):
    loop = events.get_running_loop()
    loop._remove_reader(pidfd)
    try:
        _, status = os.waitpid(pid, 0)
    except ChildProcessError:
        # The child process is already reaped
        # (may happen if waitpid() is called elsewhere).
        returncode = 255
        logger.warning(
            "child process pid %d exit status already read: "
            " will report returncode 255",
            pid)
    else:
        returncode = waitstatus_to_exitcode(status)

    os.close(pidfd)
    callback(pid, returncode, *args)

Reproducer (click to expand)

import asyncio
import os
import sys
from asyncio import events
from asyncio.unix_events import _PidfdChildWatcher
from os import waitstatus_to_exitcode
from unittest import mock


def fixed_do_wait(self, pid, pidfd, callback, args):
    loop = events.get_running_loop()
    loop._remove_reader(pidfd)
    try:
        try:
            _, status = os.waitpid(pid, 0)
        except ChildProcessError:
            returncode = 255
        else:
            returncode = waitstatus_to_exitcode(status)
    finally:
        os.close(pidfd)
    callback(pid, returncode, *args)


async def run_case(label, do_wait, exc):
    pid = os.posix_spawn(sys.executable, [sys.executable, "-c", ""], os.environ)
    pidfd = os.pidfd_open(pid)
    with mock.patch("os.waitpid", side_effect=exc):
        try:
            do_wait(_PidfdChildWatcher(), pid, pidfd, lambda *a: None, ())
        except BaseException:
            pass
    try:
        print(os.fstat(pidfd))
        leaked = True
    except OSError:
        leaked = False
    print(f"  [{label}] leaked={leaked}")
    if leaked:
        os.close(pidfd)
    os.waitpid(pid, 0)


async def main():
    print("current:")
    await run_case("OSError", _PidfdChildWatcher._do_wait, OSError("boom"))
    await run_case("KeyboardInterrupt", _PidfdChildWatcher._do_wait, KeyboardInterrupt())
    print("fixed:")
    await run_case("OSError", fixed_do_wait, OSError("boom"))
    await run_case("KeyboardInterrupt", fixed_do_wait, KeyboardInterrupt())

asyncio.run(main())

Tested on:

Distributor ID:  Ubuntu
Description:  Ubuntu 24.04.4 LTS
Release:  24.04
Codename:  noble
Kernel: 6.17.0-35-generic

Proposed fix: move os.close() into finally block:

def _do_wait(self, pid, pidfd, callback, args):
    loop = events.get_running_loop()
    loop._remove_reader(pidfd)
    try:
        try:
            _, status = os.waitpid(pid, 0)
        except ChildProcessError:
            # The child process is already reaped
            # (may happen if waitpid() is called elsewhere).
            returncode = 255
            logger.warning(
                "child process pid %d exit status already read: "
                " will report returncode 255",
                pid)
        else:
            returncode = waitstatus_to_exitcode(status)
    finally:
        os.close(pidfd)
    callback(pid, returncode, *args)

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

• gh-151179: Fix pidfd leak in asyncio _PidfdChildWatcher #151186
• [3.15] gh-151179: Fix pidfd leak in asyncio _PidfdChildWatcher (GH-151186) #157318

[deadlovelll]

i'll pin the pr