On Tue, 14 May 2024 21:30:16 GMT, Naoto Sato <na...@openjdk.org> wrote:

>> Making sure `restoreEcho` correctly reflects the state in the shutdown 
>> thread, which differs from the application's thread that issues the 
>> `readPassword()` method.
>
> Naoto Sato has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains six additional commits since 
> the last revision:
> 
>  - Merge branch 'master' into JDK-8332084-restoreEcho-readLock
>  - get/setEcho()
>  - Addresses a review comment
>  - Replaced another unused exception with _
>  - Update src/java.base/share/classes/jdk/internal/io/JdkConsoleImpl.java
>    
>    Co-authored-by: ExE Boss <3889017+exe-b...@users.noreply.github.com>
>  - initial commit

[898c609] looks robust. Separating fused `echo(boolean)` into 
`setEcho(boolean)` and `getEcho()` allows us to safely publish `final boolean 
restoreEcho` and use it as many times as we'd like without further 
synchronisation.

Even though that race between a thread that executes `readPassword` and the 
shutdown hook is still possible, it is now benign: the worst that could happen 
is that we `setEcho(true)` twice. The important thing is that we'll never do a 
wrong thing.

A general test to show that `restoreEcho` works would be good though. Thanks.

[898c609]: 
https://github.com/openjdk/jdk/pull/19184/commits/898c60938ff51fc6d6613ff54626f19e1392ab0b

-------------

Marked as reviewed by prappo (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19184#pullrequestreview-2056530026

Reply via email to