Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enable -Djline.sigcont by default in repl (so it can be suspended and resumed) #1416

Closed
scabug opened this issue Oct 11, 2008 · 16 comments
Closed

Comments

@scabug
Copy link

scabug commented Oct 11, 2008

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.

@scabug
Copy link
Author

scabug commented Oct 11, 2008

Imported From: https://issues.scala-lang.org/browse/SI-1416?orig=1
Reporter: @paulp
Attachments:

@scabug
Copy link
Author

scabug commented Oct 11, 2008

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?

@scabug
Copy link
Author

scabug commented Oct 11, 2008

@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.

@scabug
Copy link
Author

scabug commented Oct 12, 2008

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.

@scabug
Copy link
Author

scabug commented Oct 13, 2008

@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.

@scabug
Copy link
Author

scabug commented Oct 13, 2008

@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.

@scabug
Copy link
Author

scabug commented Oct 13, 2008

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.

@scabug
Copy link
Author

scabug commented Oct 15, 2008

@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.

@scabug
Copy link
Author

scabug commented Oct 18, 2008

Geoffrey Alan Washburn (washburn) said:
The patch looks good, thanks! I've applied it in r16289.

@scabug
Copy link
Author

scabug commented Feb 28, 2012

@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.

@scabug
Copy link
Author

scabug commented Feb 28, 2012

@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.

@scabug
Copy link
Author

scabug commented Jan 10, 2017

@adriaanm said:
Now we're on jline2 2.14.3 (scala/scala#5636), we should adapt the runner to enable -Djline.sigcont by default for the repl.

@SethTisue
Copy link
Member

is this still an issue in JLine 3? our JLine 3 upgrade (scala/scala#8036) will land soon, so it's only worth thinking about in that context

@SethTisue
Copy link
Member

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

@eed3si9n
Copy link
Member

Maybe of relevance:

@SethTisue
Copy link
Member

SethTisue commented Aug 21, 2019

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

@SethTisue SethTisue removed this from the Backlog milestone Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants