This is an automated email from the ASF dual-hosted git repository. ilyak pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push: new 29bddfa IGNITE-13229 Fix GridClient connection error leakage - Fixes #8006. 29bddfa is described below commit 29bddfa25edbe727dbe0edb123090a8ca815fa0a Author: zstan <stanilov...@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 <ilya.kasnach...@gmail.com> --- .../ignite/internal/commandline/Command.java | 15 ++++++++-- .../util/GridCommandHandlerAbstractTest.java | 3 ++ .../apache/ignite/util/GridCommandHandlerTest.java | 32 ++++++++++++++++++++++ .../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 +-- .../client/AdditionalSecurityCheckTest.java | 3 +- 8 files changed, 76 insertions(+), 14 deletions(-) diff --git a/modules/control-utility/src/main/java/org/apache/ignite/internal/commandline/Command.java b/modules/control-utility/src/main/java/org/apache/ignite/internal/commandline/Command.java index d65ced1..b147933 100644 --- a/modules/control-utility/src/main/java/org/apache/ignite/internal/commandline/Command.java +++ b/modules/control-utility/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/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerAbstractTest.java b/modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerAbstractTest.java index b135ed9..c996bac 100644 --- a/modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerAbstractTest.java +++ b/modules/control-utility/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/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java b/modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java index 06e7949..6174ae8 100644 --- a/modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerTest.java +++ b/modules/control-utility/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; @@ -63,6 +64,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.commandline.cache.argument.FindAndDeleteGarbageArg; @@ -121,6 +124,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; @@ -213,6 +217,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. 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/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.");