daniellansun commented on PR #2987:
URL: https://github.com/apache/groovy/pull/2987#issuecomment-6294131593
@paulk-asert Thank you for the careful read, and for checking the behaviour
locally. The three changes you asked for are in, and the documentation points
are covered as well.
## `close(Throwable)` returns `boolean`
`AsyncChannel.close(Throwable)` now returns `boolean`, the same way
`close()` does. `true` means this call closed the channel. The first close
wins, so a later call returns `false` and leaves the retained failure
unchanged. `null` is a normal close.
The default method calls `close()` and returns that result. It does not
retain the cause, so an implementation that does not override the method still
ends iteration instead of throwing. `DefaultAsyncChannel` retains the cause and
returns the same boolean from `finishClose`.
`BroadcastChannel.close(Throwable)` returns the same boolean.
`BroadcastChannel.close()` stays `void`, because that method shipped in 6.0.0.
## `merge` closes on the first failure
When one input fails, the merged channel closes with that failure. It does
not stay open until the other input ends. Values a relay has already taken, and
values already buffered on the other input, are forwarded first. The other
input is left open and is not read further. A producer that has not yet handed
a value over is not released. When both inputs fail, the first failure retained
on the output wins, and a later normal close does not replace it.
## Javadoc and the user guide
The Javadoc now describes the current contract. The note about a channel
compiled against 6.0.0 is gone; that history belongs in the commit message. The
guide says "a custom `AsyncChannel` that does not override `close(Throwable)`".
The guide also says:
- a `receive` / `send` loop must read `ChannelClosedException.getCause()`,
or the failure is dropped
- plain `for`, `each`, `collect`, and `toList` use the same iterator and
throw the failure
- a Java enhanced `for` calls `Iterator.hasNext()`, which can throw a
checked exception that `Iterator` does not declare
- a normal close and a failure close are different in `select`: a buffered
value is still delivered, and the failure is the result only when nothing
buffered remains. Which one is met first follows registration order (list
order, `fair()`, or `random()`)
The failure section no longer names C#, JavaScript, or Swift.
## Follow-ups
A `produce { }` helper would remove the try / close / catch boilerplate,
with `close(Throwable)` still the primitive underneath. I agree that is the
groovier next step, and I have left it as a follow-up so this change stays the
contract fix.
The `BroadcastChannel` subscribe / close race is a separate fix inside the
same commit. I will name it in the commit message when this lands. Suggested
sentence: "Also close the race between `BroadcastChannel.subscribe` and
`close`."
## Sonar
`volatile Throwable closeCause` is the reference publication Sonar flags as
`java:S3077`, which is what failed the reliability rating on new code. The
field is marked the same way as the other immutable holders in the tree. The
`Throwable` is not mutated after it is published.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
