ARTEMIS-3599 Removing finalization calls

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;
-      return activeMQConnectionFactory.disableFinalizeChecks();
    }
 }

artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/client/ServerLocator.java

@@ -47,16 +47,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
     *

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;
@@ -240,11 +238,6 @@ public class ClientSessionFactoryImpl implements ClientSessionFactoryInternal, C
       }
    }
 
-   @Override
-   public void disableFinalizeCheck() {
-      finalizeCheck = false;
-   }
-
    @Override
    public Lock lockFailover() {
       newFailoverLock.lock();
@@ -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

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

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;
@@ -523,11 +523,6 @@ public final class ServerLocatorImpl implements ServerLocatorInternal, Discovery
       return this;
    }
 
-   @Override
-   public void disableFinalizeCheck() {
-      finalizeCheck = false;
-   }
-
    @Override
    public ClientSessionFactoryInternal connect() throws ActiveMQException {
       return connect(false);
@@ -1310,15 +1305,6 @@ public final class ServerLocatorImpl implements ServerLocatorInternal, Discovery
       return this;
    }
 
-   @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;

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;

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;
    }

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;

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);
 

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) {

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/connection/CloseConnectionOnGCTest.java

@@ -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());
-   }
-}