sashapolo commented on a change in pull request #102:
URL: https://github.com/apache/ignite-3/pull/102#discussion_r630808725



##########
File path: 
modules/network/src/main/java/org/apache/ignite/network/internal/netty/NettyClient.java
##########
@@ -82,12 +85,34 @@ public NettyClient(
         if (clientFuture != null)
             throw new IgniteInternalException("Attempted to start an already 
started NettyClient");
 
-        clientFuture = 
NettyUtils.toChannelCompletableFuture(bootstrap.connect(address))
-            .thenApply(ch -> {
-                clientCloseFuture = 
NettyUtils.toCompletableFuture(ch.closeFuture());
-                channel = ch;
-
-                return new NettySender(channel, serializationRegistry);
+        clientFuture = new CompletableFuture<>();
+
+        NettyUtils.toChannelCompletableFuture(bootstrap.connect(address))
+            .whenComplete((channel, throwable) -> {
+                synchronized (this) {
+                    if (throwable == null) {
+                        CompletableFuture<Void> closeFuture = 
NettyUtils.toCompletableFuture(channel.closeFuture());
+
+                        if (stopped) {
+                            // Close channel in case if client has been 
stopped prior to this moment.
+                            channel.close();

Review comment:
       can be written shorter:
   ```
   // Close channel in case if client has been stopped prior to this moment.
   channel.close()
       // Wait for channel to close and then cancel the client future.
       .addListener(fut -> clientFuture.cancel(true));
   ```

##########
File path: 
modules/network/src/main/java/org/apache/ignite/network/internal/netty/NettyClient.java
##########
@@ -105,8 +130,11 @@ public NettyClient(
      *
      * @return Future that is resolved when the client's channel has closed.
      */
-    public CompletableFuture<Void> stop() {
-        channel.close();
+    public synchronized CompletableFuture<Void> stop() {
+        stopped = true;
+
+        if (channel != null)

Review comment:
       you can get rid of the `clientCloseFuture` and simply write:
   ```
   return channel == null ?
       CompletableFuture.completedFuture(null) :
       NettyUtils.toCompletableFuture(channel.close());
   ```    




-- 
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to