| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
@gpshead This seems your territory -- do you have time to look into this? The ideas seem solid. |
Sorry, something went wrong.
|
Thank you @Martiusweb for this patch. There are a lot of changes here and there are many concerns. For example the asyncio library should work with Python 3.3 and up (at least the version from this repo -- the one in the CPython repo only needs to work with the version it's part of, but we try to keep the source differences minimal). My other overarching concern is that we're already in feature freeze for Python 3.6 (3.6b1 went out a few weeks ago) and while this is technically not a new feature it certainly is a complex thing to land close to a release. You should probably also create an issue in bugs.python.org with at least a patch for subprocess.py, and a reference here. |
Sorry, something went wrong.
|
+1 to what @gvanrossum said. I can take a look at the patch in a couple of weeks (assuming you fix the CI). We can probably aim for 3.6.1 to have this. |
Sorry, something went wrong.
|
Thank you for your feedback. I pushed a commit which adds compatibility with Python 3.3. It adds a tmp_subprocess33 module which is the Popen patch backported for python 3.3. The modules tmp_subprocess and tmp_subprocess33 can probably be merged, but I guess that it can wait until we know how things will evolve on the cpython side. The CI is not yet fixed on windows (I don't have a windows setup to work on yet). I expect (...hope) that the bug which make the tests hang will be easy to fix as there is not much change in the windows code path. I also opened an issue on bugs.python.org about the changes in Popen: http://bugs.python.org/issue28287 I'm all in favor for not rushing the patch: I prefer if I can make it in python 3.6.1 or later instead of seeing it reverted because something was missing. |
Sorry, something went wrong.
|
Hello, and thanks for your contribution! Unfortunately we couldn't find an account corresponding to your GitHub username at bugs.python.org (b.p.o). If you don't already have an account at b.p.o, please create one and make sure to add your GitHub username. If you do already have an account at b.p.o then please go there and under "Your Details" add your GitHub username. And in case you haven't already, please make sure to sign the PSF contributor agreement (CLA); we can't legally look at your contribution until you have signed the CLA. Once you have done everything that's needed, please reply here and someone will verify everything is in order. |
Sorry, something went wrong.
|
Hi, I updated the PR: if my patch on Popen (submitted on b.p.o) is merged, asyncio will detect that it's able to make a NonBlockingPopen and use it, else, it will use Popen and block, but work as it used to. When it's done, I will remove the module tmp_subprocess (which monkey patches Popen) so the PR will be ready fro review. @the-knights-who-say-ni My account on b.p.o is martius, I add my github username there. I signed the CLA. Thanks ! |
Sorry, something went wrong.
|
Can you link to the specific issue you opened for your CPython patch?
|
Sorry, something went wrong.
|
Yes, this is this one: http://bugs.python.org/issue28287 |
Sorry, something went wrong.
|
Oh, so that's scheduled for Python 3.7.
|
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Dear all,
Following the bug #414, here is a patch which makes the fork + exec operation non blocking on Unix.
The patch is probably not ready to land, but I believe it's in pretty good shape.
This patch require a few changes in cpython (subprocess.Popen), which are included in this patch for discussion (in tmp_subprocess, as a child class of subprocess.Popen). Those changes have been tested with cpython unittests on my laptop. The whole patch has been tested with Python 3.5.2 on Linux, so the CI may report undetected errors for other configurations.
The problem:
subprocess.Popen() will fork and synchronously read on a pipe until it is closed as a way to wait and check the result of the exec() call in the new child process. All of this is done in the constructor of Popen.
This is an issue when the preexec_fn (user function called before exec) or exec() is slow.
This PR does the following:
This is my biggest PR for asyncio so far, and my first attempt at patching cpython, I'm eager to receive feedback and comments about the patch and about how it can successfully land in the codebase (for instance, should I get in touch with the owner of cpython's subprocess?).
Thanks for your time,
Martin