RFR: 8332084: Ensure JdkConsoleImpl.restoreEcho visibility in a shutdown hook [v5]

Pavel Rappo prappo at openjdk.org
Tue May 14 22:01:04 UTC 2024


On Tue, 14 May 2024 21:30:16 GMT, Naoto Sato <naoto at 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-Boss at 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


More information about the core-libs-dev mailing list