You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by al...@apache.org on 2020/07/15 13:49:31 UTC

[ignite] branch ignite-2.9 updated: IGNITE-13229 Fix GridClient connection error leakage - Fixes #8006.

This is an automated email from the ASF dual-hosted git repository.

alexpl pushed a commit to branch ignite-2.9
in repository https://gitbox.apache.org/repos/asf/ignite.git


The following commit(s) were added to refs/heads/ignite-2.9 by this push:
     new e910598  IGNITE-13229 Fix GridClient connection error leakage - Fixes #8006.
e910598 is described below

commit e9105981cae90625d8fb05c9ab83255c25e35c65
Author: zstan <st...@gmail.com>
AuthorDate: Tue Jul 14 17:30:57 2020 +0300

    IGNITE-13229 Fix GridClient connection error leakage - Fixes #8006.
    
    Signed-off-by: Ilya Kasnacheev <il...@gmail.com>
    (cherry picked from commit 29bddfa25edbe727dbe0edb123090a8ca815fa0a)
---
 .../apache/ignite/internal/client/GridClient.java  |  6 ++--
 .../internal/client/impl/GridClientImpl.java       |  4 +--
 .../client/impl/connection/GridClientTopology.java | 23 ++++++++++++++--
 .../client/router/impl/GridRouterClientImpl.java   |  4 +--
 .../ignite/internal/commandline/Command.java       | 15 ++++++++--
 .../client/AdditionalSecurityCheckTest.java        |  3 +-
 .../util/GridCommandHandlerAbstractTest.java       |  3 ++
 .../apache/ignite/util/GridCommandHandlerTest.java | 32 ++++++++++++++++++++++
 8 files changed, 76 insertions(+), 14 deletions(-)

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/client/GridClient.java b/modules/core/src/main/java/org/apache/ignite/internal/client/GridClient.java
index 1d8b4c7..918f98f 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/client/GridClient.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/client/GridClient.java
@@ -146,9 +146,9 @@ public interface GridClient extends AutoCloseable {
     @Override public void close();
 
     /**
-     * If client was not connected topology, throw last error encountered.
+     * Check for last topology errors.
      *
-     * @throws GridClientException If client was not connected
+     * @return {@code Exception} if client was not connected.
      */
-    public void throwLastError() throws GridClientException;
+    public GridClientException checkLastError();
 }
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/client/impl/GridClientImpl.java b/modules/core/src/main/java/org/apache/ignite/internal/client/impl/GridClientImpl.java
index 546f97a..efb924d 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/client/impl/GridClientImpl.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/client/impl/GridClientImpl.java
@@ -345,8 +345,8 @@ public class GridClientImpl implements GridClient {
     }
 
     /** {@inheritDoc} */
-    @Override public void throwLastError() throws GridClientException {
-        top.nodes();
+    @Override public GridClientException checkLastError() {
+        return top.lastError();
     }
 
     /**
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/client/impl/connection/GridClientTopology.java b/modules/core/src/main/java/org/apache/ignite/internal/client/impl/connection/GridClientTopology.java
index fc00419..c537f17 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/client/impl/connection/GridClientTopology.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/client/impl/connection/GridClientTopology.java
@@ -44,6 +44,7 @@ import org.apache.ignite.internal.client.util.GridClientUtils;
 import org.apache.ignite.internal.util.typedef.C1;
 import org.apache.ignite.internal.util.typedef.F;
 import org.apache.ignite.internal.util.typedef.internal.U;
+import org.jetbrains.annotations.Nullable;
 
 import static org.apache.ignite.internal.IgniteNodeAttributes.ATTR_MACS;
 
@@ -366,9 +367,10 @@ public class GridClientTopology {
         lock.readLock().lock();
 
         try {
-            if (lastError != null)
-                throw new GridClientDisconnectedException(
-                    "Latest topology update failed.", lastError);
+            @Nullable GridClientException e = lastError();
+
+            if (e != null)
+                throw e;
 
             return Collections.unmodifiableCollection(nodes.values());
         }
@@ -377,6 +379,21 @@ public class GridClientTopology {
         }
     }
 
+    public @Nullable GridClientException lastError() {
+        lock.readLock().lock();
+
+        try {
+            if (lastError != null)
+                return new GridClientDisconnectedException(
+                    "Latest topology update failed.", lastError);
+        }
+        finally {
+            lock.readLock().unlock();
+        }
+
+        return null;
+    }
+
     /**
      * @return Whether topology is failed.
      */
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/client/router/impl/GridRouterClientImpl.java b/modules/core/src/main/java/org/apache/ignite/internal/client/router/impl/GridRouterClientImpl.java
index 86a3d10..8a5a746 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/client/router/impl/GridRouterClientImpl.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/client/router/impl/GridRouterClientImpl.java
@@ -216,7 +216,7 @@ public class GridRouterClientImpl implements GridClient {
     }
 
     /** {@inheritDoc} */
-    @Override public void throwLastError() throws GridClientException {
-        clientImpl.throwLastError();
+    @Override public GridClientException checkLastError() {
+        return clientImpl.checkLastError();
     }
 }
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/commandline/Command.java b/modules/core/src/main/java/org/apache/ignite/internal/commandline/Command.java
index d65ced1..b147933 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/commandline/Command.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/commandline/Command.java
@@ -23,6 +23,7 @@ import java.util.logging.Logger;
 import org.apache.ignite.IgniteSystemProperties;
 import org.apache.ignite.internal.client.GridClient;
 import org.apache.ignite.internal.client.GridClientConfiguration;
+import org.apache.ignite.internal.client.GridClientException;
 import org.apache.ignite.internal.client.GridClientFactory;
 import org.apache.ignite.internal.util.typedef.F;
 import org.apache.ignite.internal.util.typedef.internal.SB;
@@ -51,8 +52,18 @@ public interface Command<T> {
         GridClient client = GridClientFactory.start(clientCfg);
 
         // If connection is unsuccessful, fail before doing any operations:
-        if (!client.connected())
-            client.throwLastError();
+        if (!client.connected()) {
+            GridClientException lastErr = client.checkLastError();
+
+            try {
+                client.close();
+            }
+            catch (Throwable e) {
+                lastErr.addSuppressed(e);
+            }
+
+            throw lastErr;
+        }
 
         return client;
     }
diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/client/AdditionalSecurityCheckTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/client/AdditionalSecurityCheckTest.java
index ad46349..e43778a 100644
--- a/modules/core/src/test/java/org/apache/ignite/internal/processors/security/client/AdditionalSecurityCheckTest.java
+++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/security/client/AdditionalSecurityCheckTest.java
@@ -84,8 +84,7 @@ public class AdditionalSecurityCheckTest extends CommonSecurityCheckTest {
             assertFalse(client.connected());
             GridTestUtils.assertThrowsAnyCause(log,
                 () -> {
-                    client.throwLastError();
-                    return null;
+                    throw client.checkLastError();
                 },
                 GridClientAuthenticationException.class,
                 "Client version is not found.");
diff --git a/modules/core/src/test/java/org/apache/ignite/util/GridCommandHandlerAbstractTest.java b/modules/core/src/test/java/org/apache/ignite/util/GridCommandHandlerAbstractTest.java
index b135ed9..c996bac 100644
--- a/modules/core/src/test/java/org/apache/ignite/util/GridCommandHandlerAbstractTest.java
+++ b/modules/core/src/test/java/org/apache/ignite/util/GridCommandHandlerAbstractTest.java
@@ -44,6 +44,7 @@ import org.apache.ignite.configuration.IgniteConfiguration;
 import org.apache.ignite.configuration.WALMode;
 import org.apache.ignite.internal.IgniteEx;
 import org.apache.ignite.internal.TestRecordingCommunicationSpi;
+import org.apache.ignite.internal.client.GridClientFactory;
 import org.apache.ignite.internal.commandline.CommandHandler;
 import org.apache.ignite.internal.processors.cache.GridCacheFuture;
 import org.apache.ignite.internal.processors.cache.distributed.dht.GridDhtTxPrepareFuture;
@@ -152,6 +153,8 @@ public abstract class GridCommandHandlerAbstractTest extends GridCommonAbstractT
         super.afterTestsStopped();
 
         cleanIdleVerifyLogFiles();
+
+        GridClientFactory.stopAll(false);
     }
 
     /** {@inheritDoc} */
diff --git a/modules/core/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java b/modules/core/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java
index 79ad548..944467a 100644
--- a/modules/core/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java
+++ b/modules/core/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java
@@ -27,6 +27,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -62,6 +63,8 @@ import org.apache.ignite.internal.GridJobExecuteResponse;
 import org.apache.ignite.internal.IgniteEx;
 import org.apache.ignite.internal.IgniteInternalFuture;
 import org.apache.ignite.internal.TestRecordingCommunicationSpi;
+import org.apache.ignite.internal.client.GridClientFactory;
+import org.apache.ignite.internal.client.impl.GridClientImpl;
 import org.apache.ignite.internal.client.util.GridConcurrentHashSet;
 import org.apache.ignite.internal.commandline.CommandHandler;
 import org.apache.ignite.internal.managers.communication.GridIoMessage;
@@ -117,6 +120,7 @@ import static org.apache.ignite.cluster.ClusterState.INACTIVE;
 import static org.apache.ignite.events.EventType.EVT_NODE_FAILED;
 import static org.apache.ignite.events.EventType.EVT_NODE_LEFT;
 import static org.apache.ignite.internal.commandline.CommandHandler.CONFIRM_MSG;
+import static org.apache.ignite.internal.commandline.CommandHandler.EXIT_CODE_CONNECTION_FAILED;
 import static org.apache.ignite.internal.commandline.CommandHandler.EXIT_CODE_INVALID_ARGUMENTS;
 import static org.apache.ignite.internal.commandline.CommandHandler.EXIT_CODE_OK;
 import static org.apache.ignite.internal.commandline.CommandHandler.EXIT_CODE_UNEXPECTED_ERROR;
@@ -208,6 +212,34 @@ public class GridCommandHandlerTest extends GridCommandHandlerClusterPerMethodAb
     }
 
     /**
+     * Test clients leakage.
+     *
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testClientsLeakage() throws Exception {
+        startGrids(1);
+
+        Map<UUID, GridClientImpl> clnts = U.field(GridClientFactory.class, "openClients");
+
+        Map<UUID, GridClientImpl> clntsBefore = new HashMap<>(clnts);
+
+        assertEquals(EXIT_CODE_OK, execute("--set-state", "ACTIVE"));
+
+        Map<UUID, GridClientImpl> clntsAfter1 = new HashMap<>(clnts);
+
+        assertTrue("Still opened clients: " + new ArrayList<>(clnts.values()), clntsBefore.equals(clntsAfter1));
+
+        stopAllGrids();
+
+        assertEquals(EXIT_CODE_CONNECTION_FAILED, execute("--set-state", "ACTIVE"));
+
+        Map<UUID, GridClientImpl> clntsAfter2 = new HashMap<>(clnts);
+
+        assertTrue("Still opened clients: " + new ArrayList<>(clnts.values()), clntsBefore.equals(clntsAfter2));
+    }
+
+    /**
      * Test enabling/disabling read-only mode works via control.sh
      *
      * @throws Exception If failed.