RFR: JDK-8215243: JShell tests failing intermitently with "Problem cleaning up the following threads:"

[Robert Field]

That makes sense.

Thumbs up.

-Robert


On 12/12/18 4:09 AM, Jan Lahoda wrote:
> Hi,
>
> After the JLine upgrade, the JShell tests tend to randomly fail with 
> errors like:
> JavaTest Message: Problem cleaning up the following threads:
> Thread-74
>   at java.base at 12-internal/java.lang.Object.wait(Native Method)
>   at java.base at 12-internal/java.lang.Object.wait(Object.java:328)
>   at 
> app/jdk.jshell at 12-internal/jdk.internal.jshell.tool.StopDetectingInputStream.read(StopDetectingInputStream.java:98)
>   at 
> app/jdk.jshell at 12-internal/jdk.internal.jshell.tool.ConsoleIOContext$TestTerminal.lambda$new$0(ConsoleIOContext.java:1212)
>   at 
> app/jdk.jshell at 12-internal/jdk.internal.jshell.tool.ConsoleIOContext$TestTerminal$$Lambda$245/0x0000000100236040.run(Unknown 
> Source)
>   at java.base at 12-internal/java.lang.Thread.run(Thread.java:835)
>
> Sadly, I didn't see that on my runs so far, but it seems to happen 
> frequently enough on the continuous builders. I was looking at the 
> StopDetectingInputStream and related classes for potential problems. 
> When the ConsoleIOContext is closed, it shutdowns the 
> StopDetectingInputStream, which sets its state to CLOSED, and the read 
> method then returns with -1. I didn't find a problem with this part.
>
> But, in StopDetecctingInputStream.write, there is:
> if (state != State.BUFFER) {
>     state = State.WAIT;
> }
>
> This could potentially un-close the StopDetectingInputStream if the 
> write method is called on a closed instance, the instance would be 
> "reopened", and if things would happen in a wrong sequence/order, the 
> "read" method could wait indefinitely, presumably leading to the 
> problem with cleaning up the thread.
>
> My proposal is to fix this by not re-setting the state CLOSED back to 
> WAIT even if write is called on a closed StopDetectingInputStream. I 
> am not sure if it will help with the problem on builders, but appears 
> to be a correct thing to do anyway.
>
> Webrev:
> http://cr.openjdk.java.net/~jlahoda/8215243/webrev.00/
> JBS:
> https://bugs.openjdk.java.net/browse/JDK-8215243
>
> What do you think?
>
> Thanks!
>
> Jan