A Terminal with custom streams and calling read/write on the thread calling readLine()

[eregon]

Hello,

I'm trying to migrate from JLine2 to JLine3 for TruffleRuby, a Ruby implementation written in parts in Java. We need to match the Ruby API of the readline standard library, and that has Readline.input=, Readline.output= so we need custom streams, and we should call Ruby methods to read/write those streams.

Those calls should happen on the same Java Thread as the caller of readLine(). That's a requirement because otherwise we would need to go multithreaded just to use JLine / the Ruby Readline API (and that would add overhead), and that's problematic because then we cannot interoperate (with GraalVM) from any Ruby REPL with languages such as JavaScript which are single-threaded and do not support multiple threads. Also, those threads are of course using extra resources.

From https://github.com/jline/jline3/wiki/Terminals That means with current JLine3 we can't use a System Terminal because those don't support using custom streams. And we can't use a Virtual Terminal because those use threads to read/write from input/output.

JLine2 allowed custom streams and with some config not use threads for reading/writing.

Do you have any advice about how to solve this? Maybe it's possible to remove the threads for a Virtual Terminal? Or maybe it's possible for PosixSysTerminal to accept custom streams?

[eregon]

Actually I just found that LineDisciplineTerminal does not seem to use threads and is a virtual terminal, I will try that.

[eregon]

LineDisciplineTerminal doesn't allow setting the InputStream so that doesn't work as-is.

FWIW interruption while reading from STDIN in the case of TruffleRuby is handled by interrupting the native read() with SIGVTALRM, so there is no need for an extra Thread.

[eregon]

Making my own subclass of AbstractTerminal that directly access the streams on the same thread seems to work, except that the input is shown twice (no JNA, no JANSI), which seems similar to https://github.com/jline/jline3/issues/30. ExternalTerminal also shows this, so non-system + no JNA + no JANSI reproduces this:

> 2 + 3
2 + 3 # duplicated output caused by a redisplay() call
5

I guess it's because setAttributes() just stores the attributes in memory for those, and so echo mode remains when JLine believes it disabled echo successfully.

I'm going to try making a subclass of AbstractPosixTerminal. But for that I need a PTY and I guess ExecPty.current() is only appropriate if the streams are stdin & stdout?

Maybe I should use the system terminal when the streams are stdin & stdout, and a virtual terminal (my subclass to avoid extra threads) otherwise.

@gnodet Do you have any tips about this issue?

[eregon]

I tried many variants of terminals (using the builder), every time I input 2+3, then left arrow, then 4:

# ExternalTerminal (no JANSI, no JNA)
ruby> 2+3^[OD4 # shows arrow keys
2+43 # prints duplicate input line
45

# PosixPtyTerminal + JANSI
>....2+3^[OD4 # broken prompt, shows arrow keys
45

the broken prompt is likely due to a (default) 0x0 terminal Size

# PosixSysTerminal + ExecPty (no JANSI, no JNA)
ruby> 2+43
45

# PosixSysTerminal + LinuxNativePty (JANSI)
ruby> 2+43
45

So virtual terminals don't seem to work with a true stdin/stdout terminal. Maybe expected? So at least it's clear if I need to handle virtual streams I need to switch between both kinds of terminals and use a system terminal when there is a real terminal.

(the system terminal warns e.g. if using < input.txt: WARNING: Unable to create a system terminal, creating a dumb terminal (enable debug logging for more information), so we need to detect that case too)

[eregon]

Something that seems to work is to use a system terminal if System.console() != null and streams are simply stdin/stdout, and otherwise create a DumbTerminal explicitly. That way avoids the warning above. And DumbTerminal does not create additional threads when passing a NonBlockingInputStream as input.

[eregon]

I managed to migrate to JLine3, but had to create 2 Terminal subclasses, 1 History subclass and 1 Parser subclass. Also needed to rework completion to set the complete flag of Candidate so an extra space wouldn't be inserted if completing mid-line.

Something that got me confused is the >.... prompt because some terminals (including DumbTerminal) have a size of 0x0 by default, which means everything get abbreviated.

Here is the full diff if anyone is interested: https://github.com/oracle/truffleruby/commit/317c565cfce626d623b5aa2fea4dd4eba87ab620

[gnodet]

Sorry for having been unresponsive the last weeks.

About the signal handling on the sys terminal, isn't the boolean flag in the constructor sufficient ? About the complete flag, setting it to false isn't working for you ? For the SingleThreadedTerminal, I think it could be put inside JLine, this looks like something reusable if you want to cope with the limitations. For the >... prompt, this is supposed to be used for terminals that can not display the required number of lines. This is disabled if the Terminal.getType() returns dumb or dumb-color. Note that this is the default value for the DumbTerminal, but if you pass xterm to a DumbTerminal, this behavior will kick in.

[eregon]

@gnodet Thanks for the reply.

About the signal handling on the sys terminal, isn't the boolean flag in the constructor sufficient ?

No, that only changes whether signals are defined eagerly in the PosixSysTerminal constructor. Setting the nativeSignals flag to true remembers the previous handlers in nativeHandlers, but that's only used by close(). So if the application needs to use or change those original signal handlers before close() it doesn't work.

In LineReaderImpl#readLine() we see:

                previousIntrHandler = terminal.handle(Signal.INT, signal -> readLineThread.interrupt());
                previousWinchHandler = terminal.handle(Signal.WINCH, this::handleSignal);
                previousContHandler = terminal.handle(Signal.CONT, this::handleSignal);

and PosixSysTerminal uses sun.misc.Signal. So what happens is on every readLine() call, if any of these 3 signal handlers was set by the application, it's lost. I think PosixSysTerminal#handle() should remember the previous (sun.misc.SignalHandler) handler when changing it, but currently it ignores the return value of Signals.registerDefault/Signals.register, and instead returns the previous entry in handlers, but that's the single (JLine) SignalHandler given to the constructor, so it doesn't really work well (and breaks if the application changes the SignalHandler for e.g. SIGINT).

About the complete flag, setting it to false isn't working for you ?

Setting it to false avoids appending/overriding the next character with a space, so that's fine. I use this logic for that: https://github.com/oracle/truffleruby/commit/317c565cfce626d623b5aa2fea4dd4eba87ab620#diff-3de2333e32c8f364e5d63f2d57867fa2R290 boolean complete = lineReader.getBuffer().cursor() == lineReader.getBuffer().length(); (i.e., only append a space if completing at the end of the line, and not mid-line) Maybe it should be the default? (In JLine2 I think this seems automatically handled)

For the SingleThreadedTerminal, I think it could be put inside JLine, this looks like something reusable if you want to cope with the limitations.

That could be nice. One limitation is the input stream needs to already be NonBlockingInputStream, or we need a way to create a NonBlockingReader reader() without a thread, even if the InputStream is not a NonBlockingInputStream. The trade-off is no built-in interruption, but maybe that matters less when using a non-interactive virtual terminal. (in TruffleRuby case we read char by char and we can interrupt a native read() so we can still interrupt but that needs some setup of course).

For the >... prompt, this is supposed to be used for terminals that can not display the required number of lines. This is disabled if the Terminal.getType() returns dumb or dumb-color. Note that this is the default value for the DumbTerminal, but if you pass xterm to a DumbTerminal, this behavior will kick in.

You're right, a type of dumb or dumb-color avoids the issue. I got the issue when making my own Terminal and inheriting e.g. the Attributes and Size from a DumbTerminal. I think a Size of 0x0 is surprising, I'd suggest to always have a non-0 default size (like in LineDisciplineTerminal).

In the output above, it also seems AbstractPosixTerminal has the same issue when using JANSI, maybe JANSI reports 0 for the number of rows?