RFR: Minor changes to `StructuredTaskScope`

[Alan Bateman]

On Mon, 11 Apr 2022 20:52:21 GMT, Paul Sandoz <psandoz at openjdk.org> wrote:

> Some suggested minor changes to `StructuredTaskScope` source while reviewing the code.
> 
> Overall I like how this approach has boiled down to about as simple as it can get while supporting the constraints of structured concurrency (thanks in no small part to `ThreadFlock`).
> 
> I think it should be possible to simplify the subclass implementations if the values of `Future.State` were comparable as in `RUNNING` < `CANCELED` < `FAILED` < `SUCCESS`. Otherwise, introducing that internally could also simplify and avoid the task scopes holding on unnecessarily to futures that are not operated on after join. I was reluctant to make such a change here but could propose as a separate PR.
> 
> I was uncertain if the `ShutdownOnSuccess.result` methods and the various exception returning/operating methods on `ShutdownOnFailure` should be constrained to throw if called before `join` has completed. Otherwise, we should specify that if operated on before `join` has completed then the results are unspecified.
> (There might be a simple way for join to return something other than `this`, thereby avoiding this issue, with some additional complexity, but I suspect you have already thought of that and took the minimal route.)
> 
> I presume there is an implicit happens-before edge on `join` that we can make explicit and specify?

Sub-class -> subclass, shutdown -> shut down, and moving the unchecked cast to FutureImpl are good - thanks. Just a few comments on the changes to the internal comments.

src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java line 273:

> 271: 
> 272:     // the set of "tracked" Future objects waiting to be returned by Future.get, created lazily
> 273:     // assigned once-only with FUTURES.compareAndSet, read by any thread

Instead of "assigned once-only ..." it might be simpler to just link to the track method

src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/StructuredTaskScope.java line 349:

> 347:     private void track(Future<?> future) {
> 348:         // create the set of Futures if not already created
> 349:         // cannot use double-checked locking since it requires use of a synchronized block

I'd prefer to leave this comment as is, only because mentioning another possible implementation might confuse reader.

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

PR: https://git.openjdk.java.net/loom/pull/142