Opened 4 years ago

Last modified 4 years ago

#9323 defect new

Cygwin: processEnded never called for spawned process

Reported by: Erik Bray Owned by: Erik Bray
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch:
Author:

Description (last modified by Erik Bray)

This issue can be reproduced with a simple test case like:

import os
from twisted.internet import reactor
from twisted.internet import utils

def printResults(*args):
    print(args)
    reactor.stop()


d = utils.getProcessOutputAndValue('echo', ['hello'], env=os.environ)
d.addCallback(printResults)
reactor.run()

The printResults callback is never run and the reactor (specifically PollReactor which is the default on Cygwin) blocks at poll().

This can also be seen with the tests:

python -m twisted.trial twisted.internet.test.test_process.ProcessTestsBuilder_PollReactorTests.test_processEnded
python -m twisted.trial twisted.internet.test.test_process.ProcessTestsBuilder_PollReactorTests.test_processExited

and possibly others, which hang due to this issue.

The problem, as I understand it, has to do with the forceReadHack flag of `ProcessWriter`, which Process always passes True.

Cygwin has a bug(?) that I documented here. The technical details in Cygwin are unimportant but in short poll()-ing for readability on the write end of a pipe does not set any useful bits (much less POLLERR). So we get stuck polling for the write end of the stdin pipe to be ready which never happens on Cygwin.

It would be better to explicitly disable the "read hack" on Cygwin, and manually call Process.closeStdin when the child process is already known to have exited.

PR: https://github.com/twisted/twisted/pull/925

Change History (3)

comment:1 Changed 4 years ago by Erik Bray

Description: modified (diff)
Keywords: review added

comment:2 Changed 4 years ago by Adi Roiban

Keywords: review removed
Owner: set to Erik Bray

Thanks for working on this. The ticket description is great and it was very easy to review the PR. Great work!

See my review comments https://github.com/twisted/twisted/pull/925#pullrequestreview-79033710

I started a conversation on the mailing list to see what can be done for patches sent for unsupported platforms. See https://twistedmatrix.com/pipermail/twisted-python/2017-November/031746.html

comment:3 Changed 4 years ago by Erik Bray

Thanks for the review. I'm also having a look at the (interesting) discussion on the mailing list. Should I follow up more here or on GitHub?

Note: See TracTickets for help on using tickets.