Issue1967

classification
Title: Can only read 8192 bytes
Type: behaviour Severity: major
Components: Core Versions: 2.5.3b2
Milestone:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: fwierzbicki Nosy List: amak, fwierzbicki, jeff.allen, kevinmcmurtrie
Priority: high Keywords:

Created on 2012-08-28.20:12:50 by kevinmcmurtrie, last changed 2013-02-07.21:00:04 by fwierzbicki.

Messages
msg7435 (view) Author: Kevin McMurtrie (kevinmcmurtrie) Date: 2012-08-28.20:16:32
org.python.core.io.StreamIO$InternalReadableByteChannel.read() can only read 8192 bytes at a time despite all other buffer settings.  This causes interactive python scripts to hang reading lines from stdin.
msg7436 (view) Author: Frank Wierzbicki (fwierzbicki) Date: 2012-08-28.20:27:17
Hi Kevin,

Thanks for the bug report - could you provide a code sample that illustrates the problem? That would greatly help us diagnose it.
msg7439 (view) Author: Kevin McMurtrie (kevinmcmurtrie) Date: 2012-08-28.20:47:37
The simplest test case it:
   new InteractiveConsole().interact();
Nothing executes until pressing ctrl-D.

It looks like the problem is caused by incorrectly stealing the code from java.nio.channels.Channels$ReadableByteChannelImpl.read().  This very critical bit is missing in 2.5.3's custom implementation:

if ((totalRead > 0) && !(in.available() > 0))
    break; // block at most once

Without this, it keeps making blocking reads until the buffer is full or the stream is closed.
msg7441 (view) Author: Frank Wierzbicki (fwierzbicki) Date: 2012-08-28.20:54:29
Hmm drat - leaving out the check for in.available() was no accident - available() is not guaranteed to give useful results and so breaks streaming in Tomcat and others. The change will likely need to happen in InteractiveConsole.
msg7444 (view) Author: Kevin McMurtrie (kevinmcmurtrie) Date: 2012-08-28.21:33:27
InputStream.available() must be at least capable of stating whether or not a non-blocking read is guaranteed.  The JVM impl in Jython 2.5.2 used available() as an optimization to possibly fill the buffer more.  If available() returned 0, it still worked in a possibly non-optimal manner.

If InputStream.available() is really broken in Tomcat, it may be better to work around the problem for only Tomcat.

ReadableByteChannel in= ... //custom Tomcat adaptor goes here
PyFile stdin= new PyFile(new StreamIO(in, true), "<stdin>", "r", 8192);

I'm actually using this to make 2.5.3 stdin work again.  What bothers me is that I don't yet know what other interactive streams (sockets?) are broken by the 2.5.3 change to StreamIO.
msg7449 (view) Author: Frank Wierzbicki (fwierzbicki) Date: 2012-08-28.22:03:39
Kevin: I share your concern - however I can't just special case Tomcat - the problem existed at least in Jetty as well - 2.5.x introduced a new IO system from 2.2 which apparently caused some problems in our servlet support in some cases - hopefully we can find a way to fix all of these input issues for 2.5.4.
msg7471 (view) Author: Kevin McMurtrie (kevinmcmurtrie) Date: 2012-10-08.18:48:28
1962 and 1972 seem to be symptoms of this bug
msg7502 (view) Author: Alan Kennedy (amak) Date: 2012-11-04.14:50:21
The code change that caused this issue is no longer necessary and should be undone.

As described in issue 1754, I have fixed the original modjy issue in a completely different way.

The code change that gave rise to this issue was checked in here

2.5: http://hg.python.org/jython/rev/4c3155645812
tip: http://hg.python.org/jython/rev/bf3a1d3eae8c

Frank, there seems to be several fixes in those checkins, so I don't want to just undo them.
msg7615 (view) Author: Jeff Allen (jeff.allen) Date: 2013-02-04.21:35:59
Frank:

I've played around with this, educating myself on io. I don't think I got any further than you, Kevin and Alan, but I don't understand what the outstanding problem is when you seemed so close.

The original motivation for the change was #1754. The changeset:
http://hg.python.org/jython/rev/bf3a1d3eae8c
introduced a custom Channel implementation intoStreamIO as a way of addressing Issue 1754. But by Alan's report that has been addressed a better way. Alan was wary of simply reversing the whole of that change as it contains some apparently useful unconnected change.

I confirmed that if I remove the custom Channel implementation from StreamIO it seems to fix the issue (and others), in 2.7 at least. Can't we just do that?

test_io_jy then fails, but I question the validity of that test, as it seems to test directly that StreamIO behaves in the way thought to be the solution to the problem, but which in fact has unwelcome side effects. I think Alan's work also provides the relevant test (of the modjy behaviour). It would be nice have this work in 2.7b as I miss InteractiveConsole (#1962) and I think it also fixes #1972. Others want it in 2.5.x, of course.
msg7617 (view) Author: Frank Wierzbicki (fwierzbicki) Date: 2013-02-04.22:06:43
There are a couple of reasons that I haven't fixed this yet - the biggest being that this change fixes something where I work and I need to make sure I get it checked with the InternalReadableByteChannel class ripped out. For this I need to roll a 2.5.4 release candidate and have it checked at work. I partly lost some of my "how to do a release" docs with the Jython wiki attack - so I just need to get all of that straight. Once that's done this should be pretty straightforward for me.
msg7642 (view) Author: Frank Wierzbicki (fwierzbicki) Date: 2013-02-07.21:00:04
Fixed in http://hg.python.org/jython/rev/ba4ec099655d Now I just need to roll a 2.5.4 rc and make sure I didn't break the app at my job :O

If I did break it then that will delay a final for sure.
History
Date User Action Args
2013-02-07 21:00:04fwierzbickisetstatus: open -> closed
resolution: fixed
messages: + msg7642
2013-02-04 22:48:41jeff.allenlinkissue1972 dependencies
2013-02-04 22:06:57fwierzbickisetpriority: high
2013-02-04 22:06:43fwierzbickisetmessages: + msg7617
2013-02-04 21:35:59jeff.allensetnosy: + jeff.allen
messages: + msg7615
2012-11-04 14:50:21amaksetmessages: + msg7502
2012-10-08 18:48:29kevinmcmurtriesetmessages: + msg7471
2012-08-28 22:07:33amaksetnosy: + amak
2012-08-28 22:03:39fwierzbickisetmessages: + msg7449
2012-08-28 21:33:28kevinmcmurtriesetmessages: + msg7444
2012-08-28 20:54:37fwierzbickisetassignee: fwierzbicki
2012-08-28 20:54:29fwierzbickisetmessages: + msg7441
2012-08-28 20:47:37kevinmcmurtriesetmessages: + msg7439
2012-08-28 20:27:17fwierzbickisetnosy: + fwierzbicki
messages: + msg7436
2012-08-28 20:16:33kevinmcmurtriesetmessages: + msg7435
2012-08-28 20:12:50kevinmcmurtriecreate