Issue1967
Created on 2012-08-28.20:12:50 by kevinmcmurtrie, last changed 2013-02-07.21:00:04 by fwierzbicki.
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.
|
|
Date |
User |
Action |
Args |
2013-02-07 21:00:04 | fwierzbicki | set | status: open -> closed resolution: fixed messages:
+ msg7642 |
2013-02-04 22:48:41 | jeff.allen | link | issue1972 dependencies |
2013-02-04 22:06:57 | fwierzbicki | set | priority: high |
2013-02-04 22:06:43 | fwierzbicki | set | messages:
+ msg7617 |
2013-02-04 21:35:59 | jeff.allen | set | nosy:
+ jeff.allen messages:
+ msg7615 |
2012-11-04 14:50:21 | amak | set | messages:
+ msg7502 |
2012-10-08 18:48:29 | kevinmcmurtrie | set | messages:
+ msg7471 |
2012-08-28 22:07:33 | amak | set | nosy:
+ amak |
2012-08-28 22:03:39 | fwierzbicki | set | messages:
+ msg7449 |
2012-08-28 21:33:28 | kevinmcmurtrie | set | messages:
+ msg7444 |
2012-08-28 20:54:37 | fwierzbicki | set | assignee: fwierzbicki |
2012-08-28 20:54:29 | fwierzbicki | set | messages:
+ msg7441 |
2012-08-28 20:47:37 | kevinmcmurtrie | set | messages:
+ msg7439 |
2012-08-28 20:27:17 | fwierzbicki | set | nosy:
+ fwierzbicki messages:
+ msg7436 |
2012-08-28 20:16:33 | kevinmcmurtrie | set | messages:
+ msg7435 |
2012-08-28 20:12:50 | kevinmcmurtrie | create | |
|