Roundup Robot added the comment:
New changeset 992ce0dcfb29 by Victor Stinner in branch '3.4':
Issue #22685: Fix test_pause_reading() of asyncio/test_subprocess
https://hg.python.org/cpython/rev/992ce0dcfb29
--
___
Python tracker
Changes by STINNER Victor victor.stin...@gmail.com:
--
status: open - closed
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22685
___
___
Roundup Robot added the comment:
New changeset 2c18dd99829c by Victor Stinner in branch '3.4':
Issue #22685: Fix test_pause_reading() of asyncio test_subprocess
https://hg.python.org/cpython/rev/2c18dd99829c
--
___
Python tracker
STINNER Victor added the comment:
Buildbot failures observed on koobs-freebsd9 and koobs-freebsd10 for 3.x and
3.4, respectively.
It looks like test_asyncio pass on the last 5 builds of the following
buildbots, and so I consider the issue as closed.
Roundup Robot added the comment:
New changeset 737355f61ba2 by Victor Stinner in branch '3.4':
Issue #22685: Debug test_pause_reading() on FreeBSD
https://hg.python.org/cpython/rev/737355f61ba2
--
___
Python tracker rep...@bugs.python.org
Roundup Robot added the comment:
New changeset 0dd91298eb17 by Victor Stinner in branch 'default':
Issue #22685, asyncio: mock also resume_reading in test_pause_reading()
https://hg.python.org/cpython/rev/0dd91298eb17
--
___
Python tracker
Roundup Robot added the comment:
New changeset 276515d2ceed by Victor Stinner in branch 'default':
Issue #22685, asyncio: resume_reading() must also be called in
test_pause_reading()
https://hg.python.org/cpython/rev/276515d2ceed
--
___
Python
STINNER Victor added the comment:
Buildbot failures observed on koobs-freebsd9 and koobs-freebsd10 for 3.x and
3.4, respectively.
I'm unable to reproduce the failure with ./python -m test -F test_asyncio or
./python -m test -F -m test_pause_reading test_asyncio on my FreeBSD 9.1 VM.
STINNER Victor added the comment:
Here is a patch with a simple unit test.
Can someone review it? If not, I will commit it without review.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22685
Guido van Rossum added the comment:
Sorry, looks good.
On Tue, Nov 25, 2014 at 6:37 AM, STINNER Victor rep...@bugs.python.org
wrote:
STINNER Victor added the comment:
Here is a patch with a simple unit test.
Can someone review it? If not, I will commit it without review.
--
Roundup Robot added the comment:
New changeset f75d40c02f0a by Victor Stinner in branch '3.4':
Closes #22685, asyncio: Set the transport of stdout and stderr StreamReader
https://hg.python.org/cpython/rev/f75d40c02f0a
New changeset 7da2288183d1 by Victor Stinner in branch 'default':
(Merge 3.4)
STINNER Victor added the comment:
Fix also pushed to Python 3.4, 3.5 and to Tulip. Thanks for the report wabu.
Tulip commit:
changeset: 1350:c3a9d355eb34
user:Victor Stinner victor.stin...@gmail.com
date:Tue Nov 25 17:17:13 2014 +0100
files: asyncio/subprocess.py
wabu added the comment:
thanks for the fixes 'n' integration
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22685
___
___
Python-bugs-list
STINNER Victor added the comment:
You can workaround the issue by setting manually the transport on the
StreamReader objects. Tell me if you need help to write the workaround if you
cannot wait for a release containing the fix.
--
___
Python
Changes by koobs koobs.free...@gmail.com:
Added file:
http://bugs.python.org/file37281/koobs-freebsd10-python34-build-598.log
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue22685
___
koobs added the comment:
Buildbot failures observed on koobs-freebsd9 and koobs-freebsd10 for 3.x and
3.4, respectively.
Both logs are attached, inlined failure below:
==
FAIL: test_pause_reading
STINNER Victor added the comment:
Here is a patch with a simple unit test.
I chose to modify the pause_reading method of the transport instead of mocking
everything to test the real code (have a better code coverage).
--
Added file:
New submission from wabu:
using `p = create_subprocess_exec(..., stdout=subprocess.PIPE, limit=...)`,
p.stdout has not transport set, so the underlying protocol is unable to pause
the reading of the transport, resulting in high memory usage when slowly
consuming input from p.stdout, even if
STINNER Victor added the comment:
p = create_subprocess_exec(..., stdout=subprocess.PIPE, limit=...)
I don't see yield from. Do you wait until the coroutine completes?
See the documentation:
https://docs.python.org/dev/library/asyncio-subprocess.html#subprocess-using-streams
Please write a
wabu added the comment:
Sorry for the confusion, yes i do the yield from. The stdout stream for the
process is actually producing data as it should. The subprocess produces a high
amount of data (pbzip2), but is only consumed slowly.
Normally when the buffer limit is reached for a stream
Guido van Rossum added the comment:
Hm... It does look like there's nothing that tells stdout (which is a
StreamReader) about its transport. Wabu, could you experiment with a change
to asyncio/subprocess.py where SubprocessStreamProtocol.connection_made()
calls
wabu added the comment:
Here's a more complete example
@coroutine
put_data(filename, queue, chunksize=16000):
pbzip2 = yield from asyncio.create_subprocess_exec(
'pbzip2', '-cd', filename,
stdout=asyncio.subprocess.PIPE, limit=self.chunksize*2)
while
Guido van Rossum added the comment:
Can you confirm that this patch fixes the problem (without you needing the
workaround in your own code)?
--
keywords: +patch
Added file: http://bugs.python.org/file36989/fix22685.patch
___
Python tracker
wabu added the comment:
thanks a lot, the fix works!
On 21.10.2014 22:16, Guido van Rossum wrote:
Guido van Rossum added the comment:
Can you confirm that this patch fixes the problem (without you needing the
workaround in your own code)?
--
keywords: +patch
Added file:
Guido van Rossum added the comment:
Victor, do you think this needs a unittest? It seems kind of difficult to
test for whether memory fills up (the machine may get wedged if it does :-).
On Tue, Oct 21, 2014 at 1:39 PM, wabu rep...@bugs.python.org wrote:
wabu added the comment:
thanks a
STINNER Victor added the comment:
Oh ok, now I understood the issue :-)
Attached patch should fix it. It implements Guido's suggestion. There is not
unit test yet.
--
Added file: http://bugs.python.org/file36990/set_transport.patch
___
Python
STINNER Victor added the comment:
test_pause_reading.py: manual test for pause reading. It should pause reading
stdout pipe 4 times on UNIX.
--
Added file: http://bugs.python.org/file36991/test_pause_reading.py
___
Python tracker
STINNER Victor added the comment:
Ok, I just missed completly the Guido posted a patch before me... Maybe we
wrote the patch at the same time, I didn't refresh my browser :-)
Victor, do you think this needs a unittest? It seems kind of difficult to
test for whether memory fills up (the
wabu added the comment:
On 21.10.2014 22:41, Guido van Rossum wrote:
Guido van Rossum added the comment:
Victor, do you think this needs a unittest? It seems kind of difficult to
test for whether memory fills up (the machine may get wedged if it does :-).
You could setup a the subprocess
Guido van Rossum added the comment:
Victor, your fix is identical to mine except for the variable names. I like
your version better. :-)
So now it's just about the unittest.
--
___
Python tracker rep...@bugs.python.org
30 matches
Mail list logo
