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