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,13 +32,15 @@ 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 AtomicBoolean closed = new AtomicBoolean( false );
 
-    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/DriverFactory.java

@@ -49,10 +49,13 @@ public final Driver newInstance( URI uri, AuthToken authToken, RoutingSettings r
         BoltServerAddress address = BoltServerAddress.from( uri );
         SecurityPlan securityPlan = createSecurityPlan( address, config );
         ConnectionPool connectionPool = createConnectionPool( authToken, securityPlan, config );
+        SessionFactory sessionFactory = createSessionFactory( config );
 
         try
         {
-            return createDriver( address, uri.getScheme(), connectionPool, config, routingSettings, securityPlan );
+            return createDriver( address, uri.getScheme(), connectionPool, config, routingSettings, securityPlan,
+                    sessionFactory
+            );
         }
         catch ( Throwable driverError )
         {
@@ -70,14 +73,16 @@ public final Driver newInstance( URI uri, AuthToken authToken, RoutingSettings r
     }
 
     private Driver createDriver( BoltServerAddress address, String scheme, ConnectionPool connectionPool,
-            Config config, RoutingSettings routingSettings, SecurityPlan securityPlan )
+            Config config, RoutingSettings routingSettings, SecurityPlan securityPlan,
+            SessionFactory sessionFactory )
     {
         switch ( scheme.toLowerCase() )
         {
         case "bolt":
-            return createDirectDriver( address, connectionPool, config, securityPlan );
+            return createDirectDriver( address, connectionPool, config, securityPlan, sessionFactory );
         case "bolt+routing":
-            return createRoutingDriver( address, connectionPool, config, routingSettings, securityPlan );
+            return createRoutingDriver( address, connectionPool, config, routingSettings, securityPlan,
+                    sessionFactory );
         default:
             throw new ClientException( format( "Unsupported URI scheme: %s", scheme ) );
         }
@@ -89,21 +94,21 @@ private Driver createDriver( BoltServerAddress address, String scheme, Connectio
      * <b>This method is package-private only for testing</b>
      */
     DirectDriver createDirectDriver( BoltServerAddress address, ConnectionPool connectionPool, Config config,
-            SecurityPlan securityPlan )
+            SecurityPlan securityPlan, SessionFactory sessionFactory )
     {
-        return new DirectDriver( address, connectionPool, securityPlan, config.logging() );
+        return new DirectDriver( address, connectionPool, securityPlan, sessionFactory, config.logging() );
     }
 
     /**
      * Creates new {@link RoutingDriver}.
      * <p>
      * <b>This method is package-private only for testing</b>
      */
-    RoutingDriver createRoutingDriver( BoltServerAddress address, ConnectionPool connectionPool,
-            Config config, RoutingSettings routingSettings, SecurityPlan securityPlan )
+    RoutingDriver createRoutingDriver( BoltServerAddress address, ConnectionPool connectionPool, Config config,
+            RoutingSettings routingSettings, SecurityPlan securityPlan, SessionFactory sessionFactory )
     {
-        return new RoutingDriver( routingSettings, address, connectionPool, securityPlan, Clock.SYSTEM,
-                config.logging() );
+        return new RoutingDriver( routingSettings, address, connectionPool, securityPlan, sessionFactory,
+                Clock.SYSTEM, config.logging() );
     }
 
     /**
@@ -122,6 +127,15 @@ ConnectionPool createConnectionPool( AuthToken authToken, SecurityPlan securityP
         return new SocketConnectionPool( poolSettings, connector, Clock.SYSTEM, config.logging() );
     }
 
+    private static SessionFactory createSessionFactory( Config config )
+    {
+        if ( LeakLoggingNetworkSessionFactory.leakLoggingEnabled() )
+        {
+            return new LeakLoggingNetworkSessionFactory( config.logging() );
+        }
+        return new NetworkSessionFactory();
+    }
+
     private static SecurityPlan createSecurityPlan( BoltServerAddress address, Config config )
     {
         try

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/test/java/org/neo4j/driver/internal/DriverFactoryTest.java

@@ -121,14 +121,14 @@ private static class ThrowingDriverFactory extends DriverFactory
 
         @Override
         DirectDriver createDirectDriver( BoltServerAddress address, ConnectionPool connectionPool, Config config,
-                SecurityPlan securityPlan )
+                SecurityPlan securityPlan, SessionFactory sessionFactory )
         {
             throw new UnsupportedOperationException( "Can't create direct driver" );
         }
 
         @Override
         RoutingDriver createRoutingDriver( BoltServerAddress address, ConnectionPool connectionPool, Config config,
-                RoutingSettings routingSettings, SecurityPlan securityPlan )
+                RoutingSettings routingSettings, SecurityPlan securityPlan, SessionFactory sessionFactory )
         {
             throw new UnsupportedOperationException( "Can't create routing driver" );
         }