spetz commented on code in PR #4096:
URL: https://github.com/apache/iggy/pull/4096#discussion_r3962005003
##########
foreign/java/java-sdk/src/main/java/org/apache/iggy/client/async/tcp/AsyncTcpConnection.java:
##########
@@ -920,18 +958,26 @@ public CompletableFuture<Void> close() {
stopHeartbeat();
releaseLoginPayload();
CompletableFuture<Void> shutdownFuture = new CompletableFuture<>();
- channelPool
- .closeAsync()
- .addListener(f ->
eventLoopGroup.shutdownGracefully().addListener(sf -> {
- if (sf.isSuccess()) {
- shutdownFuture.complete(null);
- } else {
- shutdownFuture.completeExceptionally(sf.cause());
- }
- }));
+ channelPool.closeAsync().addListener(poolClosed -> {
+ if (!ownsEventLoopGroup) {
Review Comment:
Closing the pool only closes idle channels. A pending login or logout still
holds its channel, so this branch completes `close()` while that socket and
request remain alive. I reproduced this by withholding a login reply:
`close().get()` returned, but the peer socket stayed open and the login future
remained pending. Previously, shutting down the owned group closed that channel
too. Please explicitly close this connection’s channels, including acquired
ones, before completing shutdown
##########
foreign/java/java-sdk/src/main/java/org/apache/iggy/client/async/tcp/AsyncIggyTcpClientBuilder.java:
##########
@@ -308,6 +359,28 @@ private void validateMaxVsrFrameSize() {
}
}
+ private void validateIoThreads() {
+ if (eventLoopGroup != null) {
+ return;
+ }
+ if (ioThreads < 1) {
+ throw new IggyInvalidArgumentException("IoThreads must be at least
1");
+ }
+ }
+
+ private void validateEventLoopGroup() {
+ if (eventLoopGroup == null) {
+ return;
+ }
+ if (!eventLoopGroup.isCompatible(NioIoHandle.class)) {
Review Comment:
Netty’s
[`isCompatible()`](https://github.com/netty/netty/blob/netty-4.2.17.Final/transport/src/main/java/io/netty/channel/IoEventLoopGroup.java)
calls `next()`, so validation advances the shared chooser. Pool construction,
acquisition and channel registration also advance it. I reproduced six
sequential `build()` / `connect().join()` calls on a two-thread group: all six
channels landed on the same loop. Pinning the heartbeat does not prevent this.
Please validate without advancing the chooser, then select one loop per
connection and use it for both the bootstrap and pool
--
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]
