Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

subprocess causes errors in asyncio.subprocess #114

Closed
wouterdb opened this issue Feb 20, 2019 · 2 comments
Closed

subprocess causes errors in asyncio.subprocess #114

wouterdb opened this issue Feb 20, 2019 · 2 comments
Labels

Comments

@wouterdb
Copy link

TLDR;

when combining async and sync testcases that use subprocesses, the IOloop childwatcher is not correctly closed.

workaround included

How to reproduce

import pytest
import asyncio
import subprocess


@pytest.mark.asyncio
async def test_setup(event_loop):
    # run a process using asyncio.subprocess
    process = await asyncio.subprocess.create_subprocess_exec("/usr/bin/echo", "test", stdout=subprocess.PIPE, stderr=subprocess.PIPE)
    out, err = await asyncio.wait_for(process.communicate(), timeout=30)


def test_sync(capsys):
   # generate some signals by running LOTS of processes 
    for i in range(1000):
        process = subprocess.check_output(["/usr/bin/echo", "test"], stderr=subprocess.STDOUT)

        out, err = capsys.readouterr()
        assert len(err) == 0, err
        print(out, process)

the second testcase fails if it is run after the first one

―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― test_sync ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――

capsys = <_pytest.capture.CaptureFixture object at 0x7fd991ce7b38>

    def test_sync(capsys):
        for i in range(1000):
            process = subprocess.check_output(["/usr/bin/echo", "test"], stderr=subprocess.STDOUT)
    
            out, err = capsys.readouterr()
>           assert len(err) == 0, "got unexpected errors on iterations %d\n%s"%(i, err)
E           AssertionError: got unexpected errors on iterations 278
E             Exception ignored when trying to write to the signal wakeup fd:
E             BlockingIOError: [Errno 11] Resource temporarily unavailable
E             
E           assert 125 == 0
E             -125
E             +0

test_test.py:17: AssertionError

Reason

When using asyncio.subprocess, a self-pipe is created. The python interpreter writes to this pipe whenever a signal arrives.

When the pipe is not read (e.g. when the ioloop is not running), it fills up, when it is full and new signals arrive, the interpreter panics.

If the IOloop is stopped, the pipe is detached. However, when the IOloop is replaced, the childwatcher is moved to the new ioloop. The new IOloop will then attach a new pipe.

Before pytest_asyncio stops the ioloop, it first reinstalls the old ioloop.
This causes the self-pipe to remain attached and fill up with junk, causing the interperter to panic.

Workaround

@pytest.fixture(scope="function", autouse=True)
def reset_all():
   asyncio.set_child_watcher(None)

Solution

I think the child_watcher reset could also be included in the pytest_asyncio fixtures as well

@asvetlov
Copy link
Contributor

asvetlov commented Jan 6, 2022

Python 3.8+ doesn't have this problem with the default configuration.
For 3.7 we can consider backporting of asyncio.ThreadedChildWatcher, a champion is welcome!

@seifertm
Copy link
Contributor

seifertm commented Oct 8, 2022

I can confirm that the provided example fails on CPython 3.7, but succeeds with CPython>=3.8 (tested on Linux x86_64).

CPython 3.7 will be end of life roughly 9 months from now (see PEP 537). Child watchers are currently being deprecated (see python/cpython#94597) and are scheduled for removal in Python 3.14.

If you are affected by this issue, your best course of action will likely be upgrading to a new Python version.

I'll close this issue as "won't fix" since the pytest-asyncio maintainers are unlikely to implement the backport of said child watcher. If anyone has a different opinion on the matter, please reopen the issue. We'll consider pull requests.

@seifertm seifertm closed this as not planned Won't fix, can't repro, duplicate, stale Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants