You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by cl...@apache.org on 2021/12/03 18:12:07 UTC
[activemq-artemis] branch main updated: ARTEMIS-3599 Removing finalization calls
This is an automated email from the ASF dual-hosted git repository.
clebertsuconic pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git
The following commit(s) were added to refs/heads/main by this push:
new 5730fca ARTEMIS-3599 Removing finalization calls
5730fca is described below
commit 5730fcacfa186b0fe4257d75e4c1a1b2d3c403fa
Author: Clebert Suconic <cl...@apache.org>
AuthorDate: Thu Dec 2 17:39:53 2021 -0500
ARTEMIS-3599 Removing finalization calls
---
.../cdi/factory/ConnectionFactoryProvider.java | 3 +-
.../artemis/api/core/client/ServerLocator.java | 10 --
.../core/client/impl/ClientSessionFactoryImpl.java | 18 ---
.../client/impl/ClientSessionFactoryInternal.java | 2 -
.../core/client/impl/ServerLocatorImpl.java | 31 +---
.../core/remoting/impl/netty/NettyConnector.java | 6 -
.../artemis/jms/client/ActiveMQConnection.java | 11 --
.../jms/client/ActiveMQConnectionFactory.java | 30 +---
.../server/cluster/impl/ClusterConnectionImpl.java | 1 -
.../xa/recovery/ActiveMQXAResourceWrapper.java | 1 -
.../jms/connection/CloseConnectionOnGCTest.java | 159 ---------------------
11 files changed, 4 insertions(+), 268 deletions(-)
diff --git a/artemis-cdi-client/src/main/java/org/apache/artemis/client/cdi/factory/ConnectionFactoryProvider.java b/artemis-cdi-client/src/main/java/org/apache/artemis/client/cdi/factory/ConnectionFactoryProvider.java
index de53dc0..94f1152 100644
--- a/artemis-cdi-client/src/main/java/org/apache/artemis/client/cdi/factory/ConnectionFactoryProvider.java
+++ b/artemis-cdi-client/src/main/java/org/apache/artemis/client/cdi/factory/ConnectionFactoryProvider.java
@@ -95,7 +95,6 @@ public class ConnectionFactoryProvider {
activeMQConnectionFactory.setUser(configuration.getUsername());
activeMQConnectionFactory.setPassword(configuration.getPassword());
}
- // The CF will probably be GCed since it was injected, so we disable the finalize check
- return activeMQConnectionFactory.disableFinalizeChecks();
+ return activeMQConnectionFactory;
}
}
diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/client/ServerLocator.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/client/ServerLocator.java
index 937cadf..49e6c29 100644
--- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/client/ServerLocator.java
+++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/client/ServerLocator.java
@@ -48,16 +48,6 @@ public interface ServerLocator extends AutoCloseable {
boolean isClosed();
/**
- * This method will disable any checks when a GarbageCollection happens
- * leaving connections open. The JMS Layer will make specific usage of this
- * method, since the ConnectionFactory.finalize should release this.
- * <p>
- * Warning: You may leave resources unattended if you call this method and
- * don't take care of cleaning the resources yourself.
- */
- void disableFinalizeCheck();
-
- /**
* Creates a ClientSessionFactory using whatever load balancing policy is in force
*
* @return The ClientSessionFactory
diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryImpl.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryImpl.java
index 48e70ff..be4a9cf 100644
--- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryImpl.java
+++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryImpl.java
@@ -90,8 +90,6 @@ public class ClientSessionFactoryImpl implements ClientSessionFactoryInternal, C
private ConnectorFactory connectorFactory;
- private transient boolean finalizeCheck = true;
-
private final long callTimeout;
private final long callFailoverTimeout;
@@ -241,11 +239,6 @@ public class ClientSessionFactoryImpl implements ClientSessionFactoryInternal, C
}
@Override
- public void disableFinalizeCheck() {
- finalizeCheck = false;
- }
-
- @Override
public Lock lockFailover() {
newFailoverLock.lock();
return newFailoverLock;
@@ -1037,17 +1030,6 @@ public class ClientSessionFactoryImpl implements ClientSessionFactoryInternal, C
}
}
- @Override
- protected void finalize() throws Throwable {
- if (!closed && finalizeCheck) {
- ActiveMQClientLogger.LOGGER.factoryLeftOpen(createTrace, System.identityHashCode(this));
-
- close();
- }
-
- super.finalize();
- }
-
protected ConnectorFactory instantiateConnectorFactory(final String connectorFactoryClassName) {
// Will set the instance here to avoid races where cachedFactory is set to null
diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryInternal.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryInternal.java
index 2c57c5a..45cab8a 100644
--- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryInternal.java
+++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ClientSessionFactoryInternal.java
@@ -35,8 +35,6 @@ public interface ClientSessionFactoryInternal extends ClientSessionFactory {
boolean waitForTopology(long timeout, TimeUnit unit);
- void disableFinalizeCheck();
-
String getLiveNodeId();
// for testing
diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java
index 62fbef1..1770778 100644
--- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java
+++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java
@@ -99,7 +99,8 @@ public final class ServerLocatorImpl implements ServerLocatorInternal, Discovery
private final boolean ha;
- private boolean finalizeCheck = true;
+ // this is not used... I'm only keeping it here because of Serialization compatibiity and Wildfly usage on JNDI.
+ private boolean finalizeCheck;
private boolean clusterConnection;
@@ -410,7 +411,6 @@ public final class ServerLocatorImpl implements ServerLocatorInternal, Discovery
private ServerLocatorImpl(ServerLocatorImpl locator) {
ha = locator.ha;
- finalizeCheck = locator.finalizeCheck;
clusterConnection = locator.clusterConnection;
initialConnectors = locator.initialConnectors;
discoveryGroupConfiguration = locator.discoveryGroupConfiguration;
@@ -524,11 +524,6 @@ public final class ServerLocatorImpl implements ServerLocatorInternal, Discovery
}
@Override
- public void disableFinalizeCheck() {
- finalizeCheck = false;
- }
-
- @Override
public ClientSessionFactoryInternal connect() throws ActiveMQException {
return connect(false);
}
@@ -1311,15 +1306,6 @@ public final class ServerLocatorImpl implements ServerLocatorInternal, Discovery
}
@Override
- protected void finalize() throws Throwable {
- if (finalizeCheck) {
- close();
- }
-
- super.finalize();
- }
-
- @Override
public void cleanup() {
doClose(false);
}
@@ -1774,8 +1760,6 @@ public final class ServerLocatorImpl implements ServerLocatorInternal, Discovery
for (TransportConfiguration initialConnector : initialConnectors) {
ClientSessionFactoryInternal factory = new ClientSessionFactoryImpl(ServerLocatorImpl.this, initialConnector, config, config.reconnectAttempts, threadPool, scheduledThreadPool, incomingInterceptors, outgoingInterceptors);
- factory.disableFinalizeCheck();
-
connectors.add(new Connector(initialConnector, factory));
}
}
@@ -1789,17 +1773,6 @@ public final class ServerLocatorImpl implements ServerLocatorInternal, Discovery
}
}
- @Override
- protected void finalize() throws Throwable {
- if (!isClosed() && finalizeCheck) {
- ActiveMQClientLogger.LOGGER.serverLocatorNotClosed(traceException, System.identityHashCode(this));
-
- close();
- }
-
- super.finalize();
- }
-
private final class Connector {
private final TransportConfiguration initialConnector;
diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnector.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnector.java
index b1d183a..05ed85d 100644
--- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnector.java
+++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyConnector.java
@@ -1299,12 +1299,6 @@ public class NettyConnector extends AbstractConnector {
return false;
}
- @Override
- public void finalize() throws Throwable {
- close();
- super.finalize();
- }
-
//for test purpose only
public Bootstrap getBootStrap() {
return bootstrap;
diff --git a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnection.java b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnection.java
index f2e468f..c7eb4c2 100644
--- a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnection.java
+++ b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnection.java
@@ -557,17 +557,6 @@ public class ActiveMQConnection extends ActiveMQConnectionForContextImpl impleme
return initialSession;
}
- @Override
- protected final void finalize() throws Throwable {
- if (!closed) {
- if (this.factoryReference.isFinalizeChecks()) {
- ActiveMQJMSClientLogger.LOGGER.connectionLeftOpen(creationStack);
- }
-
- close();
- }
- }
-
protected boolean isXA() {
return false;
}
diff --git a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnectionFactory.java b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnectionFactory.java
index 27bfa7d..915fee0 100644
--- a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnectionFactory.java
+++ b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnectionFactory.java
@@ -94,6 +94,7 @@ public class ActiveMQConnectionFactory extends JNDIStorable implements Connectio
private boolean cacheDestinations;
+ // keeping this field for serialization compatibility only. do not use it
private boolean finalizeChecks;
private boolean ignoreJTA;
@@ -165,16 +166,6 @@ public class ActiveMQConnectionFactory extends JNDIStorable implements Connectio
}
}
- public ActiveMQConnectionFactory disableFinalizeChecks() {
- this.finalizeChecks = false;
- return this;
- }
-
- public boolean isFinalizeChecks() {
- return finalizeChecks;
- }
-
-
@Override
public String getDeserializationBlackList() {
return deserializationBlackList;
@@ -265,8 +256,6 @@ public class ActiveMQConnectionFactory extends JNDIStorable implements Connectio
public ActiveMQConnectionFactory(final ServerLocator serverLocator) {
this.serverLocator = serverLocator;
-
- serverLocator.disableFinalizeCheck();
}
public ActiveMQConnectionFactory(final boolean ha, final DiscoveryGroupConfiguration groupConfiguration) {
@@ -275,8 +264,6 @@ public class ActiveMQConnectionFactory extends JNDIStorable implements Connectio
} else {
serverLocator = ActiveMQClient.createServerLocatorWithoutHA(groupConfiguration);
}
-
- serverLocator.disableFinalizeCheck();
}
public ActiveMQConnectionFactory(final boolean ha, final TransportConfiguration... initialConnectors) {
@@ -285,8 +272,6 @@ public class ActiveMQConnectionFactory extends JNDIStorable implements Connectio
} else {
serverLocator = ActiveMQClient.createServerLocatorWithoutHA(initialConnectors);
}
-
- serverLocator.disableFinalizeCheck();
}
@Override
@@ -950,19 +935,6 @@ public class ActiveMQConnectionFactory extends JNDIStorable implements Connectio
}
}
- @Override
- protected void finalize() throws Throwable {
- try {
- if (serverLocator != null) {
- serverLocator.close();
- }
- } catch (Exception e) {
- e.printStackTrace();
- //not much we can do here
- }
- super.finalize();
- }
-
// this may need to be set by classes which extend this class
protected void makeReadOnly() {
this.readOnly = true;
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionImpl.java
index 64939ad..9020716 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionImpl.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/impl/ClusterConnectionImpl.java
@@ -897,7 +897,6 @@ public final class ClusterConnectionImpl implements ClusterConnection, AfterConn
targetLocator.setRetryInterval(retryInterval);
}
- targetLocator.disableFinalizeCheck();
targetLocator.addIncomingInterceptor(new IncomingInterceptorLookingForExceptionMessage(manager, executorFactory.getExecutor()));
MessageFlowRecordImpl record = new MessageFlowRecordImpl(targetLocator, eventUID, targetNodeID, connector, queueName, queue);
diff --git a/artemis-service-extensions/src/main/java/org/apache/activemq/artemis/service/extensions/xa/recovery/ActiveMQXAResourceWrapper.java b/artemis-service-extensions/src/main/java/org/apache/activemq/artemis/service/extensions/xa/recovery/ActiveMQXAResourceWrapper.java
index 30fa664..7129e08 100644
--- a/artemis-service-extensions/src/main/java/org/apache/activemq/artemis/service/extensions/xa/recovery/ActiveMQXAResourceWrapper.java
+++ b/artemis-service-extensions/src/main/java/org/apache/activemq/artemis/service/extensions/xa/recovery/ActiveMQXAResourceWrapper.java
@@ -309,7 +309,6 @@ public class ActiveMQXAResourceWrapper implements XAResource, SessionFailureList
if (xaRecoveryConfig.getLocatorConfig() != null) {
serverLocator.setLocatorConfig(xaRecoveryConfig.getLocatorConfig());
}
- serverLocator.disableFinalizeCheck();
serverLocator.setProtocolManagerFactory(xaRecoveryConfig.getClientProtocolManager());
csf = serverLocator.createSessionFactory();
if (xaRecoveryConfig.getUsername() == null) {
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/connection/CloseConnectionOnGCTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/connection/CloseConnectionOnGCTest.java
deleted file mode 100644
index 427fbfb..0000000
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/connection/CloseConnectionOnGCTest.java
+++ /dev/null
@@ -1,159 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.activemq.artemis.tests.integration.jms.connection;
-
-import javax.jms.Connection;
-import javax.jms.Session;
-import java.lang.ref.WeakReference;
-import java.util.Iterator;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
-
-import org.apache.activemq.artemis.api.core.TransportConfiguration;
-import org.apache.activemq.artemis.api.jms.ActiveMQJMSClient;
-import org.apache.activemq.artemis.api.jms.JMSFactoryType;
-import org.apache.activemq.artemis.core.remoting.CloseListener;
-import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory;
-import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection;
-import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
-import org.apache.activemq.artemis.tests.util.JMSTestBase;
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Test;
-
-/**
- * A CloseConnectionOnGCTest
- */
-public class CloseConnectionOnGCTest extends JMSTestBase {
-
- private ActiveMQConnectionFactory cf;
-
- @Override
- @Before
- public void setUp() throws Exception {
- super.setUp();
-
- cf = ActiveMQJMSClient.createConnectionFactoryWithoutHA(JMSFactoryType.CF, new TransportConfiguration(INVM_CONNECTOR_FACTORY));
- cf.setBlockOnDurableSend(true);
- cf.setPreAcknowledge(true);
- }
-
- @Test
- public void testCloseOneConnectionOnGC() throws Exception {
- // Debug - don't remove this until intermittent failure with this test is fixed
- int initialConns = server.getRemotingService().getConnections().size();
-
- Assert.assertEquals(0, initialConns);
-
- Connection conn = cf.createConnection();
-
- WeakReference<Connection> wr = new WeakReference<>(conn);
-
- Assert.assertEquals(1, server.getRemotingService().getConnections().size());
- final CountDownLatch latch = new CountDownLatch(1);
- Iterator<RemotingConnection> connectionIterator = server.getRemotingService().getConnections().iterator();
- connectionIterator.next().addCloseListener(new CloseListener() {
- @Override
- public void connectionClosed() {
- latch.countDown();
- }
- });
-
- conn = null;
-
- ActiveMQTestBase.checkWeakReferences(wr);
-
- latch.await(5000, TimeUnit.MILLISECONDS);
- Assert.assertEquals(0, server.getRemotingService().getConnections().size());
- }
-
- @Test
- public void testCloseSeveralConnectionOnGC() throws Exception {
- Connection conn1 = cf.createConnection();
- Connection conn2 = cf.createConnection();
- Connection conn3 = cf.createConnection();
-
- WeakReference<Connection> wr1 = new WeakReference<>(conn1);
- WeakReference<Connection> wr2 = new WeakReference<>(conn2);
- WeakReference<Connection> wr3 = new WeakReference<>(conn3);
-
- Assert.assertEquals(3, server.getRemotingService().getConnections().size());
-
- final CountDownLatch latch = new CountDownLatch(3);
- Iterator<RemotingConnection> connectionIterator = server.getRemotingService().getConnections().iterator();
- while (connectionIterator.hasNext()) {
- RemotingConnection remotingConnection = connectionIterator.next();
- remotingConnection.addCloseListener(new CloseListener() {
- @Override
- public void connectionClosed() {
- latch.countDown();
- }
- });
- }
-
- conn1 = null;
- conn2 = null;
- conn3 = null;
-
- ActiveMQTestBase.checkWeakReferences(wr1, wr2, wr3);
-
- latch.await(5000, TimeUnit.MILLISECONDS);
-
- Assert.assertEquals(0, server.getRemotingService().getConnections().size());
- }
-
- @Test
- public void testCloseSeveralConnectionsWithSessionsOnGC() throws Exception {
- Connection conn1 = cf.createConnection();
- Connection conn2 = cf.createConnection();
- Connection conn3 = cf.createConnection();
-
- WeakReference<Connection> wr1 = new WeakReference<>(conn1);
- WeakReference<Connection> wr2 = new WeakReference<>(conn2);
- WeakReference<Connection> wr3 = new WeakReference<>(conn3);
-
- Session sess1 = conn1.createSession(false, Session.AUTO_ACKNOWLEDGE);
- Session sess2 = conn1.createSession(false, Session.AUTO_ACKNOWLEDGE);
- Session sess3 = conn2.createSession(false, Session.AUTO_ACKNOWLEDGE);
- Session sess4 = conn2.createSession(false, Session.AUTO_ACKNOWLEDGE);
- Session sess5 = conn3.createSession(false, Session.AUTO_ACKNOWLEDGE);
- Session sess6 = conn3.createSession(false, Session.AUTO_ACKNOWLEDGE);
- Session sess7 = conn3.createSession(false, Session.AUTO_ACKNOWLEDGE);
- final CountDownLatch latch = new CountDownLatch(3);
- Iterator<RemotingConnection> connectionIterator = server.getRemotingService().getConnections().iterator();
- while (connectionIterator.hasNext()) {
- RemotingConnection remotingConnection = connectionIterator.next();
- remotingConnection.addCloseListener(new CloseListener() {
- @Override
- public void connectionClosed() {
- latch.countDown();
- }
- });
- }
- sess1 = sess2 = sess3 = sess4 = sess5 = sess6 = sess7 = null;
-
- conn1 = null;
- conn2 = null;
- conn3 = null;
-
- ActiveMQTestBase.checkWeakReferences(wr1, wr2, wr3);
-
- latch.await(5000, TimeUnit.MILLISECONDS);
-
- Assert.assertEquals(0, server.getRemotingService().getConnections().size());
- }
-}