On Mon, 2 Feb 2026 22:05:03 GMT, Naoto Sato <[email protected]> wrote:
>> Fixing an issue in Console where write is blocked if other thread is waiting
>> to read, which is caused by unnecessary read/write locks. Removing those
>> would solve the problem, as the read/write synchronization is performed at
>> the StreamEn/Decoder level. One unrelated change is to refactor
>> double-checked locking with LazyConstant.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Added @requires condition
test/jdk/java/io/Console/ReadWriteBlockingTest.java line 51:
> 49: public void testReadWriteBlocking() throws Exception {
> 50: // check "expect" command availability
> 51: var expect = Paths.get("/usr/bin/expect");
Nit - in recent times we have been using `Path.of(...)` instead of
`Paths.get(...)`. `Paths.get(...)` is specified to merely invoke `Path.of(...)`.
test/jdk/java/io/Console/ReadWriteBlockingTest.java line 61:
> 59: var jdkDir = System.getProperty("test.jdk");
> 60: OutputAnalyzer output = ProcessTools.executeProcess(
> 61: "expect",
Hello Naoto, is it intentional that we check for the presence of
`/usr/bin/expect` but then rely on `PATH` when launching the `expect` process?
Should this instead be `/usr/bin/expect`?
test/jdk/java/io/Console/ReadWriteBlockingTest.java line 66:
> 64: jdkDir + "/bin/java",
> 65: "-classpath", testClasses,
> 66: "ReadWriteBlockingTest");
I think it's an oversight that the `-Djdk.console=jdk.internal.le` system
property is being set in the `@run` of this test definition instead of being
passed here to the launched `java` application. I think the test might have to
be refactored a bit to have one single `@run ....` and then here in the test
method launch the `java` main() with and without that system property.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29493#discussion_r2758247326
PR Review Comment: https://git.openjdk.org/jdk/pull/29493#discussion_r2758256127
PR Review Comment: https://git.openjdk.org/jdk/pull/29493#discussion_r2758281246
