On Fri, 16 Oct 2026 13:01:11 GMT, Viktor Klang <[email protected]> wrote:
>> Alan Bateman 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 11 additional >> commits since the last revision: >> >> - Specify fork to invoke onComplete if unable to create Thread >> - Merge branch 'master' into JDK-8391460 >> - Merge branch 'master' into JDK-8391460 >> - More API docs improvements >> - Merge branch 'master' into JDK-8391460 >> - Merge branch 'master' into JDK-8391460 >> - Improve wording >> - Merge branch 'master' into JDK-8391460 >> - Improve API docs >> - Merge branch 'master' into JDK-8391460 >> - ... and 1 more: https://git.openjdk.org/jdk/compare/a54c6172...86aeb2ca > > src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java > line 599: > >> 597: /** >> 598: * Invoked by the thread that executed a subtask after the >> subtask completes, or >> 599: * by a {@code fork} method when unable to create a thread to >> execute the subtask. > > Perhaps "Or by the owner thread when a {@code fork} method is unable to > create and start a thread to execute the subtask"? 🤔 Okay although it means change the onFork's description at the same time. > test/jdk/java/util/concurrent/StructuredTaskScope/StructuredTaskScopeTest.java > line 514: > >> 512: try (var scope = StructuredTaskScope.open(cf -> >> cf.withThreadFactory(factory))) { >> 513: // subtask1 completes >> 514: var ref = new AtomicReference<Thread>(); > > Another option to using an AtomicReference here would be to use a > SynchronousQueue<Thread> and have subtask1 put(Thread.currentThread()) and > then having the test runner thread take() > > Then we don't need the sleep-loop Yes, a SQ would be better here (and in the other test). > test/jdk/java/util/concurrent/StructuredTaskScope/StructuredTaskScopeTest.java > line 805: > >> 803: assertEquals(Subtask.State.UNAVAILABLE, subtask.state()); >> 804: assertThrows(IllegalStateException.class, subtask::get); >> 805: assertThrows(IllegalStateException.class, >> subtask::exception); > > Might make sense to move these asserts into the close()-loop (to ensure that > the transition occurs after the first invocation of close() (and doesn't > change after any subsequent invocation). That would be a more complete test, thanks. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32602#discussion_r4295448481 PR Review Comment: https://git.openjdk.org/jdk/pull/32602#discussion_r4295433606 PR Review Comment: https://git.openjdk.org/jdk/pull/32602#discussion_r4295445069
