On Thu, 23 Oct 2025 13:51:16 GMT, Alan Bateman <[email protected]> wrote:

>> Updates for JEP 525.
>> 
>> - `Joiner:onTimeout` is added  with `join` method changed to invoke this 
>> joiner method if
>>   a timeout is configured and the timeout expires before or while waiting. 
>> The `onTimeout`
>>   throws `TimeoutException` or may do nothing. This allows for `Joiner` 
>> implementation that
>>   are capable of returning a result from the subtasks that complete before 
>> the timeout expires.
>> - The `configFunction` parameter to the 3-arg `open` is changed from
>>   `Function<Configuration, Configuration>` to `UnaryOperator<Configuration>`.
>> - `Subtask::get` and `Subtask::exception` changed to consistently throw if 
>> called from
>>   any thread before the scope owner has joined.
>> - `StructuredTaskScope::join` is now specified so that it may be called 
>> again if interrupted.
>> - The parameter on `Joiner::onFork` and `Joiner::onComplete` is changed from
>>   `Subtask<? extends T` to `Subtask<T>`.
>> - `Joiner.allSuccessOrThrow` is changed to return a list of results instead 
>> of a stream of
>>   subtasks. `Joiner.allUntil` is changed to return a list of subtasks.
>> - `Joiner.anySuccessfulResultOrThrow` is renamed to `anySuccessfulOrThrow`.
>> - `Joiner.allUntil(Predicate)` is changed to allow `join` return the stream 
>> of all forked
>> subtasks when the timeout expires.
>> - `Joiner` is no longer a `@FunctionalInterface`.
>> 
>> Most of the changes are to API docs and tests. Some links are changed to use 
>> double hash mark.
>
> 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 15 additional 
> commits since the last revision:
> 
>  - Sync up from loom repo
>  - Merge branch 'master' into JDK-8367857
>  - Merge branch 'master' into JDK-8367857
>  - Sync up from loom repo
>  - Merge branch 'master' into JDK-8367857
>  - Sync up from loom repo
>  - Merge branch 'master' into JDK-8367857
>  - Merge branch 'master' into JDK-8367857
>  - Improve docs and review feedback
>  - Merge branch 'master' into JDK-8367857
>  - ... and 5 more: https://git.openjdk.org/jdk/compare/97c4ebc7...28617cff

src/java.base/share/classes/java/util/concurrent/Joiners.java line 69:

> 67:      * successfully. Cancels the scope if any subtask fails.
> 68:      */
> 69:     static final class AllSuccessful<T> implements Joiner<T, List<T>> {

It would be good to clear() the results onTimeout before throwing the 
TimeoutException.

src/java.base/share/classes/java/util/concurrent/Joiners.java line 99:

> 97:                 throw ex;
> 98:             } else {
> 99:                 return subtasks.stream().map(Subtask::get).toList();

Since the `subtasks` won't get collected until after the scope gets collected, 
we should probably clear the original collection before returning the resulting 
List.

src/java.base/share/classes/java/util/concurrent/Joiners.java line 217:

> 215:         @Override
> 216:         public List<Subtask<T>> result() {
> 217:             return List.copyOf(subtasks);

Clear the underlying collection before returning the result?

src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java line 
334:

> 332:  * #fork forking} of a subtask {@linkplain 
> java.util.concurrent##MemoryVisibility
> 333:  * <i>happen-before</i>} any actions taken by that subtask, which in turn
> 334:  * <i>happen-before</i> the subtask result is {@linkplain Subtask#get() 
> retrieved}.

This may be interpreted as users calling result() so it might be worth 
clarifying it as:

"<i>happen-before</i> the subtask result is {@linkplain Subtask#get() 
retrieved} by {@linkplain #join() join()}

src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java line 
501:

> 499:      * #onComplete(Subtask)} method defined by this interface may be 
> invoked by several
> 500:      * threads concurrently. The {@link #onTimeout()} method may be 
> invoked at around
> 501:      * the same time that subtasks complete.

Suggestion:

     * threads concurrently. The {@link #onTimeout()} method may be invoked 
concurrently
     * to subtasks completing.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27392#discussion_r2455338179
PR Review Comment: https://git.openjdk.org/jdk/pull/27392#discussion_r2455259801
PR Review Comment: https://git.openjdk.org/jdk/pull/27392#discussion_r2455264108
PR Review Comment: https://git.openjdk.org/jdk/pull/27392#discussion_r2455301159
PR Review Comment: https://git.openjdk.org/jdk/pull/27392#discussion_r2455322614

Reply via email to