r743 change to Repl.java causes freeze on load-file

[Paul Drummond]

I updated from SVN tonight and now, If I try to load a file with
errors using (load-file) Clojure seems to freeze. Files without error
load fine.

I reverted the change to Repl.java from revision 743 and the problem
goes away.

[Rich Hickey]

On Mar 12, 5:37 pm, Paul Drummond

I can't reproduce that. What errors are in the file? Where are you
running, command-line, JLine, Emacs inferior Lisp?

Also, what platform/JVM etc?

I've changed the Repl to discard from *in* on exceptions, but it is
using ready() and shouldn't block.

Rich

[Paul Drummond]

Sorry for late reply Rich - couldn't keep my eyes open any longer :)

I have just arrived at work and I can confirm the problem occurs on my
work machine (which is almost identical to my home machine so that
doesn't count for much!).

I run clojure from the command-line:

java -cp %CLOJURE_JAR% clojure.lang.Repl

I did try JLine at first but I observed strange behaviour with it so
switched back.

To see if it made a difference I tried adding JLine back in like this:

java -cp %CLOJURE_DIR%\jline-0.9.94.jar;%CLOJURE_JAR%
jline.ConsoleRunner clojure.lang.Repl

and indeed it does make a difference. It doesn't freeze with JLine!

Can you reproduce this?

FYI I am running on Windows XP Service Pack 2
java version "1.6.0_04"
Java(TM) SE Runtime Environment (build 1.6.0_04-b12)
Java HotSpot(TM) Client VM (build 10.0-b19, mixed mode, sharing)

Paul.

[Rich Hickey]

On Mar 13, 5:43 am, Paul Drummond

So far, no. The only difference in behavior I see is that sometimes
the input purge sees nothing to purge, in which case the leftover
input is there, just as before. As in the past, entering in the
closing " or ) resets things.

I need to know what the error is in the file you are trying to load.

Rich

[Paul Drummond]

I have been experimenting a bit more with this and found that there is
no need to load a file - I can reproduce from the REPL by just
entering an invalid form:

Clojure
user=> (print "hello")
hellonil
user=> (pint "hello")

>>>>>FREEZES HERE UNTIL I PRESS CTRL+Z FOLLOWED BY ENTER
^Z
java.lang.Exception: Unable to resolve symbol: pint in this context
clojure.lang.Compiler$CompilerException: REPL:2: Unable to resolve
symbol: pint in this context
at clojure.lang.Compiler.analyzeSeq(Compiler.java:3248)
at clojure.lang.Compiler.analyze(Compiler.java:3188)
at clojure.lang.Compiler.analyze(Compiler.java:3163)
at clojure.lang.Compiler.eval(Compiler.java:3268)
at clojure.lang.Repl.main(Repl.java:62)
Caused by: java.lang.Exception: Unable to resolve symbol: pint in this
context
at clojure.lang.Compiler.resolveIn(Compiler.java:3355)
at clojure.lang.Compiler.resolve(Compiler.java:3330)
at clojure.lang.Compiler.analyzeSymbol(Compiler.java:3315)
at clojure.lang.Compiler.analyze(Compiler.java:3176)
at clojure.lang.Compiler.analyze(Compiler.java:3163)
at clojure.lang.Compiler.access$100(Compiler.java:37)
at clojure.lang.Compiler$InvokeExpr.parse(Compiler.java:2344)
at clojure.lang.Compiler.analyzeSeq(Compiler.java:3243)
... 4 more
user=>

Sorry if I led you in the wrong direction with load-file - I should
have tried this earlier!

I will look into it closer after work, see if I can't get to the
bottom of it.

Cheers,
Paul.

[MikeM]

On Mar 13, 9:52 am, Paul Drummond

I have duplicated the above problem on Win XP SP2, java 1.6.0_02.

If I comment out the following in Repl.java, the exception is printed
normally.

// while(rdr.ready())
// rdr.read();

[Rich Hickey]

On Mar 13, 10:07 pm, MikeM <michael.messini...@invista.com> wrote:
> On Mar 13, 9:52 am, Paul Drummond
>
>
>
> <paul.drummond.webm...@googlemail.com> wrote:
> > I have been experimenting a bit more with this and found that there is
> > no need to load a file - I can reproduce from the REPL by just
> > entering an invalid form:
>
> > Clojure
> > user=> (print "hello")
> > hellonil
> > user=> (pint "hello")
>
> > >>>>>FREEZES HERE UNTIL I PRESS CTRL+Z FOLLOWED BY ENTER

>

> I have duplicated the above problem on Win XP SP2, java 1.6.0_02.
>
> If I comment out the following in Repl.java, the exception is printed
> normally.
>
> // while(rdr.ready())
> // rdr.read();

Can you tell if the CPU is pinned - or some other indicator it is in
an infinite loop there, or is this just an I/O anomaly?

Rich

[MikeM]

>
> Can you tell if the CPU is pinned - or some other indicator it is in
> an infinite loop there, or is this just an I/O anomaly?
>
> Rich

I ran in the Eclipse debugger: rdr.ready() returns true, rdr.read() is
called, rdr.ready() returns true again, then the thread hangs in
rdr.read(). I tried stepping into rdr.read(), but the debugger
wouldn't go there. Not sure what's happening, perhaps a native call?

I ran through this quickly, I'll try to do more in-depth debugging,
but probably not until late today.

[Rich Hickey]

On Mar 14, 8:11 am, MikeM <michael.messini...@invista.com> wrote:
> > Can you tell if the CPU is pinned - or some other indicator it is in
> > an infinite loop there, or is this just an I/O anomaly?
>
> > Rich
>
> I ran in the Eclipse debugger: rdr.ready() returns true, rdr.read() is
> called, rdr.ready() returns true again, then the thread hangs in
> rdr.read().

I was afraid of that. That's a violation of the contract for ready():

"Returns true if the next read() is guaranteed not to block for input,
false otherwise."

Rich

[christop...@gmail.com]

On Mar 14, 2:33 pm, Rich Hickey <richhic...@gmail.com> wrote:
> I was afraid of that. That's a violation of the contract for ready():
>
> "Returns true if the next read() is guaranteed not to block for input,
> false otherwise."

More in-depth analysis:
this seems to be a bug in LineNumberReader.
LineNumberReader converts \r\n into \n.
When its parent reader returns \r, LineNumberReader returns \n and set
a flag to skip the next char if it's \n.
But when you call LNR.ready it goes straight to BufferedReader.ready
which returns true because there's a \n awaiting to be read but when
you call read(), LNR read the \n but ask it's parent reader for
another char because the flag "skip \n" was previously set.

If you use exclusively LNR.readLine you don't encounter this bug
because it reads a whole line (including both \r and \n) before
returning so you are never in the buggy state.

Christophe

[christop...@gmail.com]

On Mar 14, 4:18 pm, "christo...@cgrand.net"

<christophe.gr...@gmail.com> wrote:
> If you use exclusively LNR.readLine you don't encounter this bug
> because it reads a whole line (including both \r and \n) before
> returning so you are never in the buggy state.

LNR is pretty inconsistent toward EOL conversion:
> new java.io.LineNumberReader(new java.io.StringReader("\r\n")).read() == '\n';

but :
> char[] buf = new char[1];
> new java.io.LineNumberReader(new java.io.StringReader("\r\n")).read(buf);
> buf[0] == '\r';

Hmm... I think we can work around this bug if we inhibit the EOL
conversion of LineNumberReader.
If I change the LineNumberingPushbackReader constructor this way:

public LineNumberingPushbackReader(Reader r){
super(new LineNumberReader(r) {
private char[] buf = new char[1];

public int read() throws IOException {
int n = super.read(buf);
if (n != 1) return -1;
return buf[0];
}

});
}

the REPL doesn't hang anymore.

Christophe

[Rich Hickey]

On Mar 14, 11:54 am, "christo...@cgrand.net"

<christophe.gr...@gmail.com> wrote:
> On Mar 14, 4:18 pm, "christo...@cgrand.net"
>
> <christophe.gr...@gmail.com> wrote:
> > If you use exclusively LNR.readLine you don't encounter this bug
> > because it reads a whole line (including both \r and \n) before
> > returning so you are never in the buggy state.
>

So what about:

while(rdr.ready()) rdr.readLine();

as the repl-exception input purge?

Rich

[christop...@gmail.com]

On Mar 14, 10:32 pm, Rich Hickey <richhic...@gmail.com> wrote:
> > <christophe.gr...@gmail.com> wrote:
> > > If you use exclusively LNR.readLine you don't encounter this bug
> > > because it reads a whole line (including both \r and \n) before
> > > returning so you are never in the buggy state.
>
> So what about:
>
> while(rdr.ready()) rdr.readLine();
>
> as the repl-exception input purge?

Don't know. I'll try tomorrow. As I said if you use *exclusively*
LNR.readLine you're safe, I'm not sur that readline will be able to
recover from a buggy state.

Christophe

[christop...@gmail.com]

On Mar 14, 10:41 pm, "christo...@cgrand.net"

(Sorry, I was away from keyboard when you asked me that on #clojure.)

Well... it seems to work with a simple test (entering (foo)) but I'm
worried about some corner cases: rdr.readLine() doesn't call
LNR.readLine() but LineNumberingPushbackReader.readLine which
delegates to LNR.readLine() while short-circuiting the pushbackreader.
It follows that if a char has been unread, readLine() will not return
it (as part of the line) but the subsequent call to read will.

Christophe

[christop...@gmail.com]

On Mar 15, 9:49 am, "christo...@cgrand.net"

<christophe.gr...@gmail.com> wrote:
> Well... it seems to work with a simple test (entering (foo)) but I'm
> worried about some corner cases: rdr.readLine() doesn't call
> LNR.readLine() but LineNumberingPushbackReader.readLine which
> delegates to LNR.readLine() while short-circuiting the pushbackreader.
> It follows that if a char has been unread, readLine() will not return
> it (as part of the line) but the subsequent call to read will.

I forgot to precise that LNR.readLine() blocks when called while in
the buggy state (where ready() == true but read() blocks). It seems
that, at least in my tests, the reader currently doesn't put LNR in
the buggy state before throwing an exception. Can we rely on such a
behaviour? (Thou shalt not throw an exception after reading a \n. Thou
shalt not throw an exception after unreading a character.)

While uglier, my fix (overriding the read() of LNR to ensure that
read() and read(char[]) are consistent) doesn't enforce such
constraints on the behaviour of the reader.

Christophe

[christop...@gmail.com]

Rich,

I think you can try to reproduce this misbehavior by manually entering
a carriage return at the end of the line (CTRL+V, enter).

Christophe

[Rich Hickey]

On Mar 15, 5:16 am, "christo...@cgrand.net"

The bottom line is that this is a JDK bug. I understand that calling
readLine is not an airtight workaround, but both it and the original
code should work fine. There's no way I could stomach the object-level
allocation of a buffer that should be local to read, nor do I want to
alloc a char[] every read, nor will the reasons for doing so be
evident, so I'm not going for your fix at this time, although I
understand the rationale behind it. For now, I've switched to readLine
- let's see how that works in practice.

I would greatly appreciate it if you or someone would write up and
file a bug report for LNR ready(). If you post the bug # we can all
vote for it.

Thanks,

Rich

[christop...@gmail.com]

It's not a bug, it's a feature :-<

On Mar 17, 2:10 pm, Rich Hickey <richhic...@gmail.com> wrote:
> I would greatly appreciate it if you or someone would write up and
> file a bug report for LNR ready(). If you post the bug # we can all
> vote for it.

http://bugs.sun.com/view_bug.do?bug_id=6498795

Evaluation:
The relevant portion of the LineNumberReader.read() specification is
as follows:

Read a single character. <a href="#lt">Line terminators</a> are
compressed into single newline ('\n') characters.

In contrast, this is the specification for read(char[],int, int):

Read characters into a portion of an array.

Note that the latter does not make any statements regarding line
terminator compression, it just counts them.

For compatibility reasons, we cannot change the behaviour of this
method to handle line terminators differently. Since the spec is
correct as it currently stands, this is not a bug.

[Rich Hickey]

On Mar 17, 9:31 am, "christo...@cgrand.net"

<christophe.gr...@gmail.com> wrote:
> It's not a bug, it's a feature :-<
>

The bug isn't in read - it's in ready().

ready() should not be returning true if read() will block.

Rich