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

SIGCHLD not reaped by uvloop when using subprocess protocol #128

Closed
byllyfish opened this issue Mar 8, 2018 · 3 comments
Closed

SIGCHLD not reaped by uvloop when using subprocess protocol #128

byllyfish opened this issue Mar 8, 2018 · 3 comments
Labels

Comments

@byllyfish
Copy link
Contributor

  • uvloop version: 0.9.1
  • Python version: 3.6.3
  • Platform: OS X High Sierra
  • Can you reproduce the bug with PYTHONASYNCIODEBUG in env?: Yes

I'm seeing a difference of behavior between the default asyncio event loop and uvloop when using a subprocess. When my code closes a subprocess transport under uvloop, the protocol's connection_lost handler is not called. The child process continues to show up in 'ps' output as a zombie.

The same behavior happens under ubuntu linux 16.04, with python 3.5.2.

The source code for 'test_subprocess.py' is below.

uvloop output:

$ python3 test_subprocess.py
connection_made
pipe_connection_lost 0
pipe_connection_lost 1
pipe_connection_lost 2

asyncio output:

 $ NO_UVLOOP=1 python3 test_subprocess.py
connection_made
pipe_connection_lost 0
pipe_connection_lost 1
pipe_connection_lost 2
process_exited
connection_lost

test_subprocess.py:

import asyncio
import os
import uvloop

if 'NO_UVLOOP' not in os.environ:
    asyncio.set_event_loop_policy(uvloop.EventLoopPolicy())


class Protocol(asyncio.SubprocessProtocol):

    def __init__(self):
        self.closed = asyncio.get_event_loop().create_future()

    def connection_made(self, transport):
        print('connection_made')
        
    def connection_lost(self, exc):
        print('connection_lost')
        self.closed.set_result(1)

    def pipe_connection_lost(self, fd, exc):
        print('pipe_connection_lost', fd)

    def process_exited(self):
        print('process_exited')


async def test_subprocess(loop):
    transport, protocol = await loop.subprocess_exec(Protocol, '/bin/cat')
    transport.close()
    await protocol.closed


if __name__ == '__main__':
    loop = asyncio.get_event_loop()
    loop.run_until_complete(test_subprocess(loop))
@byllyfish
Copy link
Contributor Author

If I replace the line transport.close() with any of the following, the code runs fine under uvloop:

  1. transport.kill()
  2. transport.terminate()
  3. transport.get_pipe_transport(0).close()

Looking at the code for UVProcessTransport.close (uvloop/handles/process.pyx), does self._close() close the libuv internal handle before it has had a chance to reap the child process?

    def close(self):
        if self._returncode is None:
            self._kill(uv.SIGKILL)

        if self._stdin is not None:
            self._stdin.close()
        if self._stdout is not None:
            self._stdout.close()
        if self._stderr is not None:
            self._stderr.close()

        self._close()

uvloop Issue #64 ("asyncio.get_child_watcher Not Working" ) is similar to this one. I think there's a race condition in the sample code provided, which is why it may not have been reproducible.

@1st1
Copy link
Member

1st1 commented Mar 14, 2018

Yeah, there's definitely a bug here. I don't have time to look at this right now 😞, but feel free to submit a PR.

@1st1
Copy link
Member

1st1 commented May 23, 2018

Well, one obvious solution is to simply remove self._close() line. The handler will be properly closed in its __dealloc__. I'm going to try this.

@1st1 1st1 added the bug label May 23, 2018
@1st1 1st1 closed this as completed in #151 May 25, 2018
1st1 added a commit that referenced this issue May 25, 2018
* Fix subprocess.close() to let its processes die gracefully

Fixes #128.
1st1 added a commit that referenced this issue May 25, 2018
A follow up to fixes for #128.
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

2 participants