On Thu, 14 Nov 2024 19:16:34 GMT, Brian Burkhalter <[email protected]> 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