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

JLine hangs reading line in GitBash #245

Open
bdw429s opened this issue May 11, 2016 · 21 comments
Open

JLine hangs reading line in GitBash #245

bdw429s opened this issue May 11, 2016 · 21 comments

Comments

@bdw429s
Copy link
Contributor

bdw429s commented May 11, 2016

I tried out my CommandBox library which uses JLine on GitShell today. The shell prompt works fine as well as me typing text, but as soon as I hit enter, the program hangs for a very long time (I've never waited to see if it completes), consuming a large amount of CPU usage and an ever-growing amount of memory.

Pulling a stack trace from the JVM shows the thread is on this same place every time. It would appear as though some sort of error is being thrown over and over and getting caught every time.

I am using Windows 8 and here is where I downloaded Git shell from:
https://git-for-windows.github.io/

   java.lang.Thread.State: RUNNABLE
    at java.lang.Throwable.fillInStackTrace(Native Method)
    at java.lang.Throwable.fillInStackTrace(Unknown Source)
    - locked <0x00000000e71d05e8> (a java.io.IOException)
    at java.lang.Throwable.<init>(Unknown Source)
    at java.lang.Exception.<init>(Unknown Source)
    at java.io.IOException.<init>(Unknown Source)
    at org.fusesource.jansi.internal.Kernel32.readConsoleInputHelper(Kernel32.java:765)
    at org.fusesource.jansi.internal.Kernel32.readConsoleKeyInput(Kernel32.java:794)
    at org.fusesource.jansi.internal.WindowsSupport.readConsoleInput(WindowsSupport.java:97)
    at jline.WindowsTerminal.readConsoleInput(WindowsTerminal.java:214)
    at jline.WindowsTerminal.access$000(WindowsTerminal.java:54)
    at jline.WindowsTerminal$1.read(WindowsTerminal.java:156)
    at jline.internal.NonBlockingInputStream.read(NonBlockingInputStream.java:166)
    - locked <0x00000000ae441ed0> (a jline.internal.NonBlockingInputStream)
    at jline.internal.NonBlockingInputStream.read(NonBlockingInputStream.java:135)
    at jline.internal.NonBlockingInputStream.read(NonBlockingInputStream.java:243)
    at jline.internal.InputStreamReader.read(InputStreamReader.java:257)
    - locked <0x00000000ae441ed0> (a jline.internal.NonBlockingInputStream)
    at jline.internal.InputStreamReader.read(InputStreamReader.java:194)
    - locked <0x00000000ae441ed0> (a jline.internal.NonBlockingInputStream)
    at jline.console.ConsoleReader.readCharacter(ConsoleReader.java:2135)
    at jline.console.ConsoleReader.readCharacter(ConsoleReader.java:2125)
    at jline.console.ConsoleReader.readBinding(ConsoleReader.java:2210)
    at jline.console.ConsoleReader.readLine(ConsoleReader.java:2450)
    at jline.console.ConsoleReader.readLine(ConsoleReader.java:2362)
    at jline.console.ConsoleReader.readLine(ConsoleReader.java:2350)
    at jline.console.ConsoleReader.readLine(ConsoleReader.java:2338)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
    at java.lang.reflect.Method.invoke(Unknown Source)
    at lucee.runtime.reflection.pairs.MethodInstance.invoke(MethodInstance.java:55)
    at lucee.runtime.reflection.Reflector.callMethod(Reflector.java:857)
    at lucee.runtime.util.VariableUtilImpl.callFunctionWithoutNamedValues(VariableUtilImpl.java:756)
    at lucee.runtime.PageContextImpl.getFunction(PageContextImpl.java:1590)
    at system.shell_cfc$cf.udfCall2(C:\Users\Brad\.CommandBox\cfml\system\Shell.cfc:409)
...

Let me know if this should actually be logged in JAnsi.

@gnodet
Copy link
Member

gnodet commented May 13, 2016

Any idea what git bash uses under the cover ?
I know in jline3, the support for cygwin is different from the plain windows use case.
I suppose this may be the same.

@bdw429s
Copy link
Contributor Author

bdw429s commented May 13, 2016

No, not right off. I don't actually use Gitshell, but the issue was reported to me from one of my CommandBox users.

@acrisci
Copy link

acrisci commented May 13, 2016

I'm getting the same issue in cygwin, although without a huge increase in cpu usage reported earlier.

When I exit out of my console with ctrl-c, it seems to replay whatever I typed into the console in my bash terminal.

@acrisci
Copy link

acrisci commented May 13, 2016

Nevermind, the workaround in #1194 adding -Djline.terminal=jline.UnixTerminal resolved my issue.

@bdw429s
Copy link
Contributor Author

bdw429s commented Jun 8, 2016

See this related CommandBox ticket: https://ortussolutions.atlassian.net/browse/COMMANDBOX-395

@bdw429s
Copy link
Contributor Author

bdw429s commented Aug 7, 2016

@gnodet Any new ideas on this one?

@bdw429s
Copy link
Contributor Author

bdw429s commented Aug 10, 2016

See this ticket: #259

@radai-rosenblatt
Copy link

(copied over from dup #259)
im running an application using jline from git bash on windows 7. the application source code does this:

Terminal terminal = TerminalFactory.create();
ConsoleReader reader = new ConsoleReader("whatever", System.in, System.out, terminal);
int read = reader.readCharacter();

when run as-is the terminal returned from the factory call is a windows terminal, and it fails to read anything. after CTRL-C is hit and the program exists all the keystrokes typed are passed onto bash (who returns with a "command not found") indicating theyre probably never consumed.

when i try manually creating a UnixTerminal:

Terminal terminal = new UnixTerminal();
terminal.init();
//rest as before

the characters are returned (read) only when i hit enter (so i can read lines but not single keystrokes).

git bash version:

$ uname -a
MINGW64_NT-6.1 latitude 2.5.0(0.295/5/3) 2016-03-31 18:47 x86_64 Msys
$ echo $0
/usr/bin/bash

@swimmesberger
Copy link

swimmesberger commented Nov 29, 2016

The problem seems to be in:
https://github.com/fusesource/jansi-native/blob/master/src/main/java/org/fusesource/jansi/internal/Kernel32.java#L811

The "ReadConsoleInputW" function returns "0" which according to "https://msdn.microsoft.com/en-us/library/ms684961(v=VS.85).aspx" is a errornous case. The exact error is not retrieved via the "GetLastError" function instead a simple IOException is thrown which will be catched and logged with debug level here https://github.com/jline/jline2/blob/master/src/main/java/jline/WindowsTerminal.java#L224. Because the events variable is null at this point a empty byte[0] will be returned which at this point https://github.com/jline/jline2/blob/master/src/main/java/jline/WindowsTerminal.java#L163 lead to a endless loop because "bufIdx" is zero and the returned array is zero.

A better approach would probably be to somehow get the correct error of jansi (via. GetLastError) and throw a RuntimeException pointing out the internal error.

@bdw429s
Copy link
Contributor Author

bdw429s commented Nov 29, 2016

Thanks for the research @swimmesberger. To clarify, using getLastError() won't necessarily fix the issue, it will just show the actual error that's happening right?

@gnodet @chirino Does this give us something to go off of? I've still got CommandBox users who are complaining to me that they can't use GitBash so I'd love to be part of a fix here.

@swimmesberger
Copy link

swimmesberger commented Nov 29, 2016

@bdw429s no this is by no means a fix - GetLastError() would simply give us a windows specific error code which could further clarify what is happening and possibly lead to a fix. I have encountered this specific issue when debugging in eclipse so this isn't potentially the same exact issue here. But we have definitely a common issue here because the error handling seems suboptimal, there can be potentially multiple issues which lead to a unsuccessful "ReadConsoleInput" call. Fact is every time something goes wrong in a native call Windows can't recover from we end up in a endless loop with nothing more than a debug log output of a unspecific IOException.

@bdw429s
Copy link
Contributor Author

bdw429s commented Dec 2, 2016

@gnodet Do you know if this issue has been fixed in JLine3?

@gnodet
Copy link
Member

gnodet commented Dec 2, 2016

The jansi issue is clearly not fixed, and looking at the jline 3 code, i think the endless loop would still happen. Is there an easy way to reproduce the problem or does simply running jline in git bash is the way ?

@gnodet
Copy link
Member

gnodet commented Dec 2, 2016

I've done some testing on jline3 to support mingw, it seems to work correctly when using the unix style terminal after fixing the automatic detection code.

@gnodet
Copy link
Member

gnodet commented Dec 2, 2016

Could you try using a UnixTerminal by setting the jline.terminal system property to unix ?
It seems to work for me.

@bdw429s
Copy link
Contributor Author

bdw429s commented Dec 2, 2016

@gnodet To be clear, should I force the setting to unix in every case-- even for my WIndows or Mac users? What is the purpose of the jline.terminal setting if I need it to always have the same value? Or were you just suggesting a temporary change to test with?

@gnodet
Copy link
Member

gnodet commented Dec 2, 2016

No, the unix setting can be used when running on unix, or when running on windows inside cygwin or mingw. The default discovery mechanism does not support cygwin/mingw, so it uses the windows terminal which afaik only works when running in the real windows console.

@bdw429s
Copy link
Contributor Author

bdw429s commented Dec 2, 2016

@gnodet Interesting. The CommandBox CLI is a tool that can be used on any OS using any shell so I can't make that decision for the user at compile time. For example, I don't know if my Windows users will use DOS, Cygwin, GitBash, or something else. I basically distribute CommandBox as an exe file (on Windows) and users do what they want with it. The fact that it even uses java is an abstracted implementation detail, so I'm not going to ask my users to configure JVM args based on how they use it. Are you saying that I'll need to write my own shell detection for this? I was hoping we could get it smart enough to "just work" as they say :)

@gnodet
Copy link
Member

gnodet commented Dec 2, 2016

Yes, the detection should be automatic, but jline2 has never really supported cigwin/mingw environment, hence the false discovery (it works for detecting the OS though). You could mimic the jline3 detection mechanism. You could also create a patch and raise a PR, I could have a look.

@bdw429s
Copy link
Contributor Author

bdw429s commented Dec 2, 2016

@gnodet Thanks again for the reply. I reviewed your recent commit in JLine3 (jline/jline3@85e44ed). Does that mean that this will work now automatically in JLine3? I also saw a change to your gogo scripts, but I'm unclear on whether that is also required for the fix to function (It looks like the corresponding bat file for Windows was left untouched). For what it's worth, I'm packaging the Windows version of my JLine-enabled application with Launch4J so I don't think I have the ability to customize the script that it starts with, but I can set Java system properties manually if I need to from Java when it starts.

Anyway, just struggling a little to know what needs to change. If this is going to work out of the box in JLine3, it's just another reason for me to upgrade! If I'll still need some custom detection for this to work even on JLine3, I'm still a little hazy on what I need to do.

@gnodet
Copy link
Member

gnodet commented Dec 3, 2016

Yes, it should work automatically with JLine3.
The script change is minor and does not actually change anything because the detection is done, but no particular action is performed. It is needed for cygwin to change the path from unix to windows, but mingw does the change itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants