paulk-asert commented on PR #2987:
URL: https://github.com/apache/groovy/pull/2987#issuecomment-6304583191
Looks good to me overall. Close-with-cause is opt-in, so a plain `close()`
behaves exactly as before, and it reaches iteration, the
`filter`/`map`/`split`/`tap` pipelines, `merge`, the Reactive Streams bridge
(`onError`) and `select`.
Checked locally, merged with current master:
- `groovy.concurrent.*`, `*Async*` and `*Channel*` tests: 1846 run, 0
failures.
- `AsyncChannelFailureTest`, `BroadcastChannelTest` and
`BroadcastChannelAsPublisherTest`: 8 forced reruns, no flakes.
**One thing to fix before merging: a value can be lost in `merge`.** In
`AsyncChannel.relay` (`src/main/java/groovy/concurrent/AsyncChannel.java`,
around line 391):
```java
if (self.isStopping() && !pending.isDone()) {
pending.cancel();
break;
}
```
A sender can commit its transfer to `pending` between the `isDone()` check
and `cancel()`. The claim then refuses the cancel (`cancel()` returns `false`),
but we `break` anyway, so a value already removed from the input is dropped.
The sender's `send` has completed successfully, so nobody sees an error. That
contradicts the `merge` Javadoc ("every value a relay has already taken … has
been forwarded").
Suggested fix: only break when the cancel actually wins, and otherwise fall
through to the `await` so the value is forwarded:
```java
if (self.isStopping() && !pending.isDone() && pending.cancel()) {
break;
}
```
The next iteration then exits through the `isStopping() && getBufferedSize()
== 0` check as usual. It's hard to hit in a test, but it's a one-line change.
Not blocking, just noting: when one input fails, `merge` keeps forwarding
the other input's buffered values, so closing the output can wait a while
behind a fast producer on that input. The docs describe this ("every value
already buffered on the other input").
--
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]
