On Thu, 21 Aug 2025 03:10:23 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> Maybe there is a cleaner overall solution that doesn't require locking >> `this` at all... ? >> >> For example... just brainstorming, this is not fully baked and probably >> outside of the scope of this PR... >> * Use `AtomicReference.getAndUpdate()` to set `inputCharset`, >> `outputCharset`, and `errorCharset` and check for conflicting values >> * Use `StableValue` for creating `inputReader`, `outputWriter`, and >> `errorWriter` (using the chosen charset) >> * Make `closed` an `AtomicBoolean` and use `compareAndSwap()` to make >> `close()` idempotent >> * In `close()`, obtain `inputReader`, `outputWriter`, and `errorWriter` from >> their `StableValue`s and just forcibly close them. We may unnecessarily >> create a transient reader or writer(s) but so what. >> >> I think it's worthwhile to minimize locking in `Process` because it's more >> likely than most classes to be accessed by multiple threads at once. > > Removing the locking breaks compatibility in the same way. You can argue the > use of `synchronized(this)` is not specified anywhere, but people can become > aware of it and depend on it. The CSR request would have to determine whether > this is acceptable or not. What David said is right although it doesn't really make sense to extend Process, at least not outside the JDK. If the API were introduced today then it wouldn't have a public constructor, or maybe it would be a sealed hierarchy. It's possible of course that that there are Process implementations for testing or delegation but unlikely a "real implementation". ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26649#discussion_r2289921607
