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

[Pavel Rappo]

On Tue, 21 May 2024 22:51:14 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 incrementally with one additional commit since the last revision:
> 
>   Used a dedicate lock for restoreEcho

I looked at the most recent commit, https://github.com/openjdk/jdk/pull/19184/commits/f69f747a8669647d6f924369fd98b945f9d0ae63.

You are right, the [race][] that we hypothesised previously when `restoreEcho` was `volatile` is no longer present. However, there's another race. If it's of any consolation, that new race isn't new at all: it was there before. (I know it's frustrating to be discussing the same problem over and over again, but we're making progress.)

What we want to do is restore the echo state immediately before JVM exits. We achieve this by installing a shutdown hook which restores the state. However, there's no coordination between the shutdown hook and any thread that also modifies the state.

If I read [this][shutdown-sequence] correctly, due to the mechanics of JVM exit, we simply don't know which thread finishes first: a thread that calls `readPassword` or the shutdown hook. If the shutdown hook finishes first, then a `readPassword` thread can corrupt the state: unlike the shutdown hook, which JVM _normally has to wait to complete_, the `readPassword` thread can be terminated at any moment. It might as well be terminated before `finally` but after `echo(false)`, in which case we end up with echo turned off.

What I'm saying is that we should ensure that the shutdown hook has the final say: once completed, no one should modify echo status. I understand that this means more involved communication between threads.

Does it make sense?

[shutdown-sequence]: https://docs.oracle.com/en/java/javase/22/docs/api/java.base/java/lang/Runtime.html#shutdown
[race]: https://github.com/openjdk/jdk/pull/19184#issuecomment-2109779741

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

PR Comment: https://git.openjdk.org/jdk/pull/19184#issuecomment-2125137216