Merge pull request #285 from lutovich/1.1-remove-finalize

Removed finalize from NetworkSession

Author: Zhen Li

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 ( config.logLeakedSessions() )
+        {
+            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,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.neo4j.driver.internal.spi.Connection;
+import org.neo4j.driver.v1.Logger;
+import org.neo4j.driver.v1.Logging;
+import org.neo4j.driver.v1.Session;
+
+class LeakLoggingNetworkSessionFactory implements SessionFactory
+{
+    private static final String LOGGER_NAME = "sessionLeak";
+
+    private final Logger logger;
+
+    LeakLoggingNetworkSessionFactory( Logging logging )
+    {
+        this.logger = logging.getLog( LOGGER_NAME );
+    }
+
+    @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;
+
+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;
+
+interface SessionFactory
+{
+    Session newInstance( Connection connection );
+}