RFR: JDK-8202105: jshell tool: on exiting, terminal echo is disabled

[Xueming Shen]

On 04/25/2018 01:39 PM, Jan Lahoda wrote:
> So, if I understand correctly, it would be:
> boolean flipEcho;
> and the readPassword would do something like:
> if (echo0() != false) {

if (echo0()) { ...

>     flipEcho = true;
>     echo(false);
> }
> ....
> if (flipEcho) { //this would also be in the shutdown hook
>      echo(!echo0());
> }

if (flipEcho) {
     echo(true);
     flipEcho = false;
}
?

>
> I guess I can do that (the variant with two boolean feels to me slightly easier to understand and a tiny bit more robust, but this flip boolean variant is doable as well).
>
> Thanks,
>     Jan
>
> On 25.4.2018 21:07, Martin Buchholz wrote:
>> I keep hoping for something simpler.
>>
>> Is it possible to have more than one Console object?  Looks like NO.
>> Assuming no, then you simply need one static flag
>>
>> boolean restoreEcho;
>>
>> (it could also be an instance field of Console - that would be slightly
>> more principled)
>>
>> In readPassword you check current value of echo and set restoreEcho if it
>> was changed.  Shutdown hook also checks the same restoreEcho.
>>
>> For bonus points, only create the shutdown hook the first time readPassword
>> is called with echo on to appease the Emacs shell users with pitchforks.
>>
>> On Wed, Apr 25, 2018 at 9:34 AM, Xueming Shen <xueming.shen at oracle.com>
>> wrote:
>>
>>> On 4/25/18, 9:02 AM, Martin Buchholz wrote:
>>>
>>> It would be more correct I think for Console to track if there is a
>>> pending readPassword in progress and try to restore echo on exit only if
>>> so.  But a little annoying to implement (need an additional boolean?)
>>>
>>>
>>> I think that is what Jan proposed "to add restoredEchoOnShutdown = true",
>>> and I'm fine
>>> with that. I'm just a little confused why jshell invokes System.console()
>>> if it handles all "raw"
>>> terminal stuff itself.
>>>
>>>
>>> On Wed, Apr 25, 2018 at 8:52 AM, Xueming Shen <xueming.shen at oracle.com>
>>> wrote:
>>>
>>>>
>>>> Hi Jan,
>>>>
>>>> I saw System.console() returns null inside jshell ... but it seems there
>>>> are 2 vms.
>>>> I would assume jshell  itself sets the terminal to raw and then call
>>>> System.console()?
>>>> for example an alternative for this issue is to ask jshell's impl to call
>>>> System.console()
>>>> before going into raw mode? No, I'm not saying the proposed one is not a
>>>> good one,
>>>> just wanted to make sure I understand the situation correctly.
>>>>
>>>> Thanks,
>>>> Sherman
>>>>
>>>>
>>>>
>>>> On 4/25/18, 4:50 AM, Jan Lahoda wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Under:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8194750
>>>>>
>>>>> j.i.Console was changed to capture the state of the terminal echo at
>>>>> creation time, and to restore it on shutdown.
>>>>>
>>>>> That is problematic at least in jshell, where the terminal is already in
>>>>> the raw mode when j.i.Console is created, and so "echo disabled" is
>>>>> recorded there. So even though jshell itself sets the terminal into the
>>>>> original mode when it terminates, the shutdown hook in j.i.Console, which
>>>>> is run later, sets the echo to off even if it was enabled before the VM
>>>>> started.
>>>>>
>>>>> My understanding is that the shutdown hook is only needed in case the VM
>>>>> goes down while readPassword is running. So I tried to change the shutdown
>>>>> hook to only work while readPassword is running.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8202105
>>>>> Webrev: http://cr.openjdk.java.net/~jlahoda/8202105/webrev.00/
>>>>>
>>>>> What do you think?
>>>>>
>>>>> Thanks,
>>>>>      Jan
>>>>>
>>>>
>>>>
>>>
>>>