Copilot commented on code in PR #6862:
URL: https://github.com/apache/ignite-3/pull/6862#discussion_r2473031611
##########
modules/client/src/main/java/org/apache/ignite/internal/client/TcpClientChannel.java:
##########
@@ -443,14 +446,20 @@ private <T> CompletableFuture<T> send(
}
}
- private <T> CompletableFuture<T> completeAsync(
+ private <T> void completeAsync(
@Nullable PayloadReader<T> payloadReader,
@Nullable CompletableFuture<PayloadInputChannel> notificationFut,
- ClientMessageUnpacker unpacker
+ ClientMessageUnpacker unpacker,
+ @Nullable Throwable err,
+ CompletableFuture<T> resFut
) {
- try {
- CompletableFuture<T> resFut = new CompletableFuture<>();
+ if (err != null) {
+ assert unpacker == null : "unpacker must be null if err is not
null";
+ asyncContinuationExecutor.execute(() ->
resFut.completeExceptionally(ViewUtils.ensurePublicException(err)));
Review Comment:
If `asyncContinuationExecutor.execute()` throws a
`RejectedExecutionException` (e.g., if the executor is shut down), the
exception will propagate uncaught and `resFut` will never be completed.
Consider wrapping this in a try-catch block to handle executor failures and
complete the future exceptionally in the catch block.
```suggestion
try {
asyncContinuationExecutor.execute(() ->
resFut.completeExceptionally(ViewUtils.ensurePublicException(err)));
} catch (Throwable execErr) {
resFut.completeExceptionally(ViewUtils.ensurePublicException(execErr));
}
```
##########
modules/client/src/main/java/org/apache/ignite/internal/client/TcpClientChannel.java:
##########
@@ -460,11 +469,9 @@ private <T> CompletableFuture<T> completeAsync(
resFut.completeExceptionally(ViewUtils.ensurePublicException(t));
}
});
-
- return resFut;
- } catch (Throwable t) {
+ } catch (Throwable execErr) {
unpacker.close();
- return failedFuture(ViewUtils.ensurePublicException(t));
+ asyncContinuationExecutor.execute(() ->
resFut.completeExceptionally(ViewUtils.ensurePublicException(execErr)));
Review Comment:
If `asyncContinuationExecutor.execute()` throws an exception on line 474
(e.g., `RejectedExecutionException`), the exception will propagate uncaught and
`resFut` will never be completed. This creates a risk of hanging futures.
Consider falling back to `resFut.completeExceptionally()` directly if the
executor submission fails.
```suggestion
try {
asyncContinuationExecutor.execute(() ->
resFut.completeExceptionally(ViewUtils.ensurePublicException(execErr)));
} catch (Throwable execErr2) {
resFut.completeExceptionally(ViewUtils.ensurePublicException(execErr2));
}
```
--
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]
