RFR: 8343039: Remove jdk.internal.misc.InternalLock and usages from java.io [v9]

Chen Liang liach at openjdk.org
Fri Nov 15 01:22:53 UTC 2024


On Thu, 14 Nov 2024 19:16:34 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> Uses of `InternalLock` are removed and `synchronized` is reinstated.
>
> Brian Burkhalter 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 nine additional commits since the last revision:
> 
>  - Merge
>  - 8343039: Change "resizble" comment in BOS
>  - 8343039: Remove failing getDeclaredField call from test java/lang/ProcessBuilder/Basic.java
>  - 8343039: Remove java.base/jdk.internal.misc from @modules in test java/lang/ProcessBuilder/Basic.java
>  - 8343039: Remove InternalLock reference from java/lang/ProcessBuilder/Basic.java
>  - 8343039: Address reviewer comments
>  - 8343039: Remove JavaIOPrint{Stream,Writer}Access and the use thereof
>  - 8343039: Remove use of InternalLock from Stream{De,En}coder and throwable; remove InternalLock class; address comments on BIS and BOS
>  - 8343039: Remove jdk.internal.misc.InternalLock and usages from java.io

Looks good.  Aside from the output stream smaller buffer from virtual threads, everything is pretty much a restoration to JDK18's state (aside from trivial whitespace changes)

src/java.base/share/classes/java/io/BufferedReader.java line 329:

> 327:             if (term != null) term[0] = false;
> 328: 
> 329:             bufferLoop:

How do we usually handle the indentation of labels?  I personally prefer this type of indentation, but in the jdk18 code the label has one less level of indentation, so it aligns with the enclosing `}`.  Don't know if we are looking for parity with 18 code.

test/jdk/java/lang/ProcessBuilder/Basic.java line 2677:

> 2675:                 else unexpected(t);}}
> 2676: 
> 2677:     static boolean isLocked(BufferedInputStream bis) throws Exception {

The original version in JDK 18 uses arbitrary Object and a configurable number of millis.  That said, this can stay as is as it's functionally the same as the original 18 test.

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

Marked as reviewed by liach (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22048#pullrequestreview-2437027840
PR Review Comment: https://git.openjdk.org/jdk/pull/22048#discussion_r1842785851
PR Review Comment: https://git.openjdk.org/jdk/pull/22048#discussion_r1843070312


More information about the core-libs-dev mailing list