Closed scabug closed 5 years ago
Imported From: https://issues.scala-lang.org/browse/SI-1416?orig=1 Reporter: @paulp Attachments:
Geoffrey Alan Washburn (washburn) said: The patch looks mostly okay. But are you certain that it won't throw away IOExceptions caused by something other than an interrupted system call?
@paulp said: I am not certain. I was going to write it to check against the message ("Interrupted system call", I think) but I was afraid that would vary between platforms or be unstable across versions. I think if the IOException gets to the point where I'm discarding it, the interpreter user has already demonstrated their indifference to it.
Geoffrey Alan Washburn (washburn) said: Okay, looking at the code more closely I am also a bit worried that it might be possible to cause the interpreter to go into an infinite loop if some other kind of IO error occurs, because readLine will just keep looping in that case.
It might be better to contact the JLine people first and see what they think about the problem, because I am inclined to believe that this is a bug in their code.
Incidentally, I just noticed that this does not seem to be a problem under Linux, but I can reproduce the problem on MacOS X.
@paulp said: I've had quite a difficult time assigning blame here. What documentation I can find says that SA_RESTART has been standard on OS X for a while, so applications should have to go out of their way not to have system calls restarted. Ctrl-Z to the interpreter sends SIGTSTP, which causes the read call to return EINTR. The differing behavior between linux and OS X (which I'm on if that's not obvious) goes back to who automatically restarts interrupted read calls.
What's a little uncool here is why the JVM doesn't specify the behavior, given that as far as I know you can't manipulate signals from java without JNI. If I had to pick a villain I'd go with apple's red-headed JVM.
Best I can do for the time being is attached, which confirms the exception message is exactly the one I get on OSX. Won't win any generality awards but should be unlikely to do any harm, and might solve the issue for all mac users.
@paulp said: Aha - soylatte can be suspended without modification, so it's pretty definitely apple's jvm. Unfortunately their v6 java has the same behavior as their v5.
Geoffrey Alan Washburn (washburn) said: Thanks for doing all this research. I just verified that the same problem also occurs even without using JLine, so it would seem that we cannot really assign blame to them either, and it might make sense to pull this out into InteractiveReader somehow. It could possibly be slightly more portable to change the check to instead check the JVM vendor rather than the exact message string.
@paulp said: Okay, one last patch I hope. This makes the interpreter suspendable with or without jline. So far I have been trying to patch from a "modify as little as possible even if the approach is suboptimal" perspective, but this required touching a couple more files.
Geoffrey Alan Washburn (washburn) said: The patch looks good, thanks! I've applied it in r16289.
@paulp said: Reopening because this has regressed. In fact I can't find it working at all in a released version. I know without a shred of doubt it worked at the time this patch was integrated; I don't know for how much longer.
@paulp said: In the current codebase, the IOExceptions which would trigger a terminal reset never reach the top level. I believe they are being swallowed in the version of jline we are now using. But I don't think I can explain the fact that this doesn't work in 2.8 the same way, so the whole theory may be flawed.
@adriaanm said:
Now we're on jline2 2.14.3 (https://github.com/scala/scala/pull/5636), we should adapt the runner to enable -Djline.sigcont
by default for the repl.
is this still an issue in JLine 3? our JLine 3 upgrade (https://github.com/scala/scala/pull/8036) will land soon, so it's only worth thinking about in that context
is this even still an issue in JLine 2? I was not able to reproduce the problem in any Scala version I tried (going back to 2.7.x) just now, on MacOS
on Gitter, @nogurenn says he isn't available to reproduce it either (on MacOS). we can reopen the ticket if anyone can confirm that the problem still exists on any modern setup
my informal impression is that people used to complain about this but I haven't heard a complaint about it in ages
Ctrl-Z on the interpreter is effectively the same as Ctrl-C because jline throws an uncaught java.io.IOException when the job is resumed. This is a huge bummer when one has work in progress in the interpreter. Attached patch catches and discards the exception; ctrl-C and ctrl-D still exit the interpreter.