Removed finalize from NetworkSession

It contained logic for closing opened underlying connection and logging info
about the leak. Closing of resources in finalize is not reliable and overridden
finalize can hurt when many sessions are garbage collected.

Commit also introduces a system property `org.neo4j.driver.logSessionLeaks`
to enable logging of unclosed/leaked sessions and log stacktrace of where they
were instantiated. This is meant to help with session leak investigations. So
finalization overhead will be present only when problem is present and is
being investigated.

Author: lutovich

driver/src/main/java/org/neo4j/driver/internal/BaseDriver.java

@@ -32,14 +32,16 @@ abstract class BaseDriver implements Driver
     private final static String DRIVER_LOG_NAME = "Driver";
 
     private final SecurityPlan securityPlan;
+    protected final SessionFactory sessionFactory;
     protected final Logger log;
 
     private final ReentrantReadWriteLock closedLock = new ReentrantReadWriteLock();
     private boolean closed;
 
-    BaseDriver( SecurityPlan securityPlan, Logging logging )
+    BaseDriver( SecurityPlan securityPlan, SessionFactory sessionFactory, Logging logging )
     {
         this.securityPlan = securityPlan;
+        this.sessionFactory = sessionFactory;
         this.log = logging.getLog( DRIVER_LOG_NAME );
     }
 

driver/src/main/java/org/neo4j/driver/internal/DirectDriver.java

@@ -36,17 +36,18 @@ public DirectDriver(
             BoltServerAddress address,
             ConnectionPool connections,
             SecurityPlan securityPlan,
+            SessionFactory sessionFactory,
             Logging logging )
     {
-        super( securityPlan, logging );
+        super( securityPlan, sessionFactory, logging );
         this.address = address;
         this.connections = connections;
     }
 
     @Override
     protected Session newSessionWithMode( AccessMode mode )
     {
-        return new NetworkSession( connections.acquire( address ) );
+        return sessionFactory.newInstance( connections.acquire( address ) );
     }
 
     @Override

driver/src/main/java/org/neo4j/driver/internal/LeakLoggingNetworkSession.java

@@ -0,0 +1,65 @@
+/**
+ * Copyright (c) 2002-2016 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed 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.neo4j.driver.internal;
+
+import org.neo4j.driver.internal.spi.Connection;
+import org.neo4j.driver.v1.Logger;
+
+import static java.lang.System.lineSeparator;
+
+class LeakLoggingNetworkSession extends NetworkSession
+{
+    private final Logger log;
+    private final String stackTrace;
+
+    LeakLoggingNetworkSession( Connection connection, Logger log )
+    {
+        super( connection );
+        this.log = log;
+        this.stackTrace = captureStackTrace();
+    }
+
+    @Override
+    protected void finalize() throws Throwable
+    {
+        logLeakIfNeeded();
+        super.finalize();
+    }
+
+    private void logLeakIfNeeded()
+    {
+        if ( isOpen() )
+        {
+            log.error( "Neo4j Session object leaked, please ensure that your application" +
+                       "calls the `close` method on Sessions before disposing of the objects.\n" +
+                       "Session was create at:\n" + stackTrace, null );
+        }
+    }
+
+    private static String captureStackTrace()
+    {
+        StringBuilder result = new StringBuilder();
+        StackTraceElement[] elements = Thread.currentThread().getStackTrace();
+        for ( StackTraceElement element : elements )
+        {
+            result.append( "\t" ).append( element ).append( lineSeparator() );
+        }
+        return result.toString();
+    }
+}

driver/src/main/java/org/neo4j/driver/internal/LeakLoggingNetworkSessionFactory.java

@@ -0,0 +1,49 @@
+/**
+ * Copyright (c) 2002-2016 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed 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.neo4j.driver.internal;
+
+import org.neo4j.driver.internal.spi.Connection;
+import org.neo4j.driver.v1.Logger;
+import org.neo4j.driver.v1.Logging;
+import org.neo4j.driver.v1.Session;
+
+public class LeakLoggingNetworkSessionFactory implements SessionFactory
+{
+    private static final String SYSTEM_PROPERTY_NAME = "org.neo4j.driver.logSessionLeaks";
+    private static final String LOGGER_NAME = "sessionLeak";
+
+    private final Logger logger;
+
+    public LeakLoggingNetworkSessionFactory( Logging logging )
+    {
+        this.logger = logging.getLog( LOGGER_NAME );
+    }
+
+    public static boolean leakLoggingEnabled()
+    {
+        String value = System.getProperty( SYSTEM_PROPERTY_NAME );
+        return Boolean.parseBoolean( value );
+    }
+
+    @Override
+    public Session newInstance( Connection connection )
+    {
+        return new LeakLoggingNetworkSession( connection, logger );
+    }
+}

driver/src/main/java/org/neo4j/driver/internal/NetworkSession.java

@@ -67,7 +67,7 @@ public void run()
     private ExplicitTransaction currentTransaction;
     private AtomicBoolean isOpen = new AtomicBoolean( true );
 
-    public NetworkSession( Connection connection )
+    NetworkSession( Connection connection )
     {
         this.connection = connection;
 
@@ -126,6 +126,7 @@ public static StatementResult run( Connection connection, Statement statement )
         return cursor;
     }
 
+    @Override
     public synchronized void reset()
     {
         ensureSessionIsOpen();
@@ -255,18 +256,6 @@ private void ensureConnectionIsValidBeforeOpeningTransaction()
         ensureConnectionIsOpen();
     }
 
-    @Override
-    protected void finalize() throws Throwable
-    {
-        if ( isOpen.compareAndSet( true, false ) )
-        {
-            logger.error( "Neo4j Session object leaked, please ensure that your application calls the `close` " +
-                          "method on Sessions before disposing of the objects.", null );
-            connection.close();
-        }
-        super.finalize();
-    }
-
     private void ensureNoUnrecoverableError()
     {
         if ( connection.hasUnrecoverableErrors() )

driver/src/main/java/org/neo4j/driver/internal/NetworkSessionFactory.java

@@ -0,0 +1,31 @@
+/**
+ * Copyright (c) 2002-2016 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed 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.neo4j.driver.internal;
+
+import org.neo4j.driver.internal.spi.Connection;
+import org.neo4j.driver.v1.Session;
+
+public class NetworkSessionFactory implements SessionFactory
+{
+    @Override
+    public Session newInstance( Connection connection )
+    {
+        return new NetworkSession( connection );
+    }
+}

driver/src/main/java/org/neo4j/driver/internal/RoutingDriver.java

@@ -51,18 +51,19 @@ public RoutingDriver(
             BoltServerAddress seedAddress,
             ConnectionPool connections,
             SecurityPlan securityPlan,
+            SessionFactory sessionFactory,
             Clock clock,
             Logging logging )
     {
-        super( verifiedSecurityPlan( securityPlan ), logging );
+        super( verifiedSecurityPlan( securityPlan ), sessionFactory, logging );
         this.loadBalancer = new LoadBalancer( settings, clock, log, connections, seedAddress );
     }
 
     @Override
     protected Session newSessionWithMode( AccessMode mode )
     {
         Connection connection = acquireConnection( mode );
-        NetworkSession networkSession = new NetworkSession( connection );
+        Session networkSession = sessionFactory.newInstance( connection );
         return new RoutingNetworkSession( networkSession, mode, connection.boltServerAddress(), loadBalancer );
     }
 

driver/src/main/java/org/neo4j/driver/internal/SessionFactory.java

@@ -0,0 +1,27 @@
+/**
+ * Copyright (c) 2002-2016 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed 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.neo4j.driver.internal;
+
+import org.neo4j.driver.internal.spi.Connection;
+import org.neo4j.driver.v1.Session;
+
+public interface SessionFactory
+{
+    Session newInstance( Connection connection );
+}

driver/src/main/java/org/neo4j/driver/v1/GraphDatabase.java

@@ -24,17 +24,17 @@
 
 import org.neo4j.driver.internal.ConnectionSettings;
 import org.neo4j.driver.internal.DirectDriver;
-import org.neo4j.driver.internal.NetworkSession;
+import org.neo4j.driver.internal.LeakLoggingNetworkSessionFactory;
+import org.neo4j.driver.internal.NetworkSessionFactory;
 import org.neo4j.driver.internal.RoutingDriver;
+import org.neo4j.driver.internal.SessionFactory;
 import org.neo4j.driver.internal.net.BoltServerAddress;
 import org.neo4j.driver.internal.net.pooling.PoolSettings;
 import org.neo4j.driver.internal.net.pooling.SocketConnectionPool;
 import org.neo4j.driver.internal.security.SecurityPlan;
-import org.neo4j.driver.internal.spi.Connection;
 import org.neo4j.driver.internal.spi.ConnectionPool;
 import org.neo4j.driver.internal.util.Clock;
 import org.neo4j.driver.v1.exceptions.ClientException;
-import org.neo4j.driver.v1.util.Function;
 
 import static java.lang.String.format;
 import static org.neo4j.driver.internal.security.SecurityPlan.insecure;
@@ -47,17 +47,6 @@
  */
 public class GraphDatabase
 {
-
-    private static final Function<Connection,Session>
-            SESSION_PROVIDER = new Function<Connection,Session>()
-    {
-        @Override
-        public Session apply( Connection connection )
-        {
-            return new NetworkSession( connection );
-        }
-    };
-
     /**
      * Return a driver for a Neo4j instance with the default configuration settings
      *
@@ -182,16 +171,20 @@ public static Driver driver( URI uri, AuthToken authToken, Config config )
         // And finally, construct the driver proper
         ConnectionPool connectionPool =
                 new SocketConnectionPool( connectionSettings, securityPlan, poolSettings, config.logging() );
+
+        SessionFactory sessionFactory = createSessionFactory( config );
+
         switch ( scheme.toLowerCase() )
         {
         case "bolt":
-            return new DirectDriver( address, connectionPool, securityPlan, config.logging() );
+            return new DirectDriver( address, connectionPool, securityPlan, sessionFactory, config.logging() );
         case "bolt+routing":
             return new RoutingDriver(
                     config.routingSettings(),
                     address,
                     connectionPool,
                     securityPlan,
+                    sessionFactory,
                     Clock.SYSTEM,
                     config.logging() );
         default:
@@ -244,4 +237,13 @@ private static SecurityPlan createSecurityPlan( BoltServerAddress address, Confi
             return insecure();
         }
     }
+
+    private static SessionFactory createSessionFactory( Config config )
+    {
+        if ( LeakLoggingNetworkSessionFactory.leakLoggingEnabled() )
+        {
+            return new LeakLoggingNetworkSessionFactory( config.logging() );
+        }
+        return new NetworkSessionFactory();
+    }
 }

driver/src/test/java/org/neo4j/driver/internal/LeakLoggingNetworkSessionFactoryTest.java

@@ -0,0 +1,42 @@
+/**
+ * Copyright (c) 2002-2016 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed 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.neo4j.driver.internal;
+
+import org.junit.Test;
+
+import org.neo4j.driver.internal.spi.Connection;
+import org.neo4j.driver.v1.Logging;
+import org.neo4j.driver.v1.Session;
+
+import static org.hamcrest.Matchers.instanceOf;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.mock;
+
+public class LeakLoggingNetworkSessionFactoryTest
+{
+    @Test
+    public void createsLeakLoggingNetworkSessions()
+    {
+        SessionFactory factory = new LeakLoggingNetworkSessionFactory( mock( Logging.class ) );
+
+        Session session = factory.newInstance( mock( Connection.class ) );
+
+        assertThat( session, instanceOf( LeakLoggingNetworkSession.class ) );
+    }
+}