Merge pull request #362 from lutovich/1.2-fix-log-leak

Fix possible logger leak

Author: zhenlineo

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

@@ -18,11 +18,10 @@
  */
 package org.neo4j.driver.internal;
 
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import org.neo4j.driver.internal.logging.DelegatingLogger;
 import org.neo4j.driver.internal.retry.RetryLogic;
 import org.neo4j.driver.internal.spi.Connection;
 import org.neo4j.driver.internal.spi.ConnectionProvider;
@@ -47,6 +46,8 @@
 
 public class NetworkSession implements Session, SessionResourcesHandler
 {
+    private static final String LOG_NAME = "Session";
+
     private final ConnectionProvider connectionProvider;
     private final AccessMode mode;
     private final RetryLogic retryLogic;
@@ -64,7 +65,7 @@ public NetworkSession( ConnectionProvider connectionProvider, AccessMode mode, R
         this.connectionProvider = connectionProvider;
         this.mode = mode;
         this.retryLogic = retryLogic;
-        this.logger = logging.getLog( "Session-" + hashCode() );
+        this.logger = new DelegatingLogger( logging.getLog( LOG_NAME ), String.valueOf( hashCode() ) );
     }
 
     @Override
@@ -378,28 +379,4 @@ private void closeCurrentConnection( boolean sync )
             logger.debug( "Released connection " + connection.hashCode() );
         }
     }
-
-    private static List<Throwable> recordError( Throwable error, List<Throwable> errors )
-    {
-        if ( errors == null )
-        {
-            errors = new ArrayList<>();
-        }
-        errors.add( error );
-        return errors;
-    }
-
-    private static void addSuppressed( Throwable error, List<Throwable> suppressedErrors )
-    {
-        if ( suppressedErrors != null )
-        {
-            for ( Throwable suppressedError : suppressedErrors )
-            {
-                if ( error != suppressedError )
-                {
-                    error.addSuppressed( suppressedError );
-                }
-            }
-        }
-    }
 }

driver/src/main/java/org/neo4j/driver/internal/logging/DelegatingLogger.java

@@ -0,0 +1,97 @@
+/*
+ * Copyright (c) 2002-2017 "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.logging;
+
+import org.neo4j.driver.v1.Logger;
+
+import static java.util.Objects.requireNonNull;
+
+public class DelegatingLogger implements Logger
+{
+    private final Logger delegate;
+    private final String messagePrefix;
+
+    public DelegatingLogger( Logger delegate )
+    {
+        this( delegate, null );
+    }
+
+    public DelegatingLogger( Logger delegate, String messagePrefix )
+    {
+        this.delegate = requireNonNull( delegate );
+        this.messagePrefix = messagePrefix;
+    }
+
+    @Override
+    public void error( String message, Throwable cause )
+    {
+        delegate.error( messageWithPrefix( message ), cause );
+    }
+
+    @Override
+    public void info( String message, Object... params )
+    {
+        delegate.info( messageWithPrefix( message ), params );
+    }
+
+    @Override
+    public void warn( String message, Object... params )
+    {
+        delegate.warn( messageWithPrefix( message ), params );
+    }
+
+    @Override
+    public void debug( String message, Object... params )
+    {
+        if ( isDebugEnabled() )
+        {
+            delegate.debug( messageWithPrefix( message ), params );
+        }
+    }
+
+    @Override
+    public void trace( String message, Object... params )
+    {
+        if ( isTraceEnabled() )
+        {
+            delegate.trace( messageWithPrefix( message ), params );
+        }
+    }
+
+    @Override
+    public boolean isTraceEnabled()
+    {
+        return delegate.isTraceEnabled();
+    }
+
+    @Override
+    public boolean isDebugEnabled()
+    {
+        return delegate.isDebugEnabled();
+    }
+
+    private String messageWithPrefix( String message )
+    {
+        if ( messagePrefix == null )
+        {
+            return message;
+        }
+        return String.format( "[%s] %s", messagePrefix, message );
+    }
+}

driver/src/main/java/org/neo4j/driver/internal/net/SocketConnection.java

@@ -25,6 +25,7 @@
 import java.util.Queue;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import org.neo4j.driver.internal.logging.DelegatingLogger;
 import org.neo4j.driver.internal.messaging.InitMessage;
 import org.neo4j.driver.internal.messaging.Message;
 import org.neo4j.driver.internal.messaging.RunMessage;
@@ -47,6 +48,8 @@
 
 public class SocketConnection implements Connection
 {
+    private static final String LOG_NAME = "Connection";
+
     private final Queue<Message> pendingMessages = new LinkedList<>();
     private final SocketResponseHandler responseHandler;
     private final AtomicBoolean isInterrupted = new AtomicBoolean( false );
@@ -57,7 +60,7 @@ public class SocketConnection implements Connection
 
     SocketConnection( BoltServerAddress address, SecurityPlan securityPlan, int timeoutMillis, Logging logging )
     {
-        Logger logger = logging.getLog( "Connection-" + hashCode() );
+        Logger logger = new DelegatingLogger( logging.getLog( LOG_NAME ), String.valueOf( hashCode() ) );
         this.socket = new SocketClient( address, securityPlan, timeoutMillis, logger );
         this.responseHandler = createResponseHandler( logger );
 

driver/src/main/java/org/neo4j/driver/internal/net/pooling/BlockingPooledConnectionQueue.java

@@ -27,6 +27,7 @@
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import org.neo4j.driver.internal.logging.DelegatingLogger;
 import org.neo4j.driver.internal.net.BoltServerAddress;
 import org.neo4j.driver.internal.spi.PooledConnection;
 import org.neo4j.driver.internal.util.Supplier;
@@ -39,6 +40,8 @@
  */
 public class BlockingPooledConnectionQueue
 {
+    public static final String LOG_NAME = "ConnectionQueue";
+
     /** The backing queue, keeps track of connections currently in queue */
     private final BlockingQueue<PooledConnection> queue;
     private final Logger logger;
@@ -162,6 +165,6 @@ private void disposeSafely( PooledConnection connection )
 
     private static Logger createLogger( BoltServerAddress address, Logging logging )
     {
-        return logging.getLog( "ConnectionQueue[" + address + "]" );
+        return new DelegatingLogger( logging.getLog( LOG_NAME ), address.toString() );
     }
 }

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

@@ -36,7 +36,6 @@
 import org.neo4j.driver.v1.Session;
 
 import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.startsWith;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Matchers.any;
@@ -83,7 +82,7 @@ public void logsMessageWithStacktraceDuringFinalizationIfLeaked() throws Excepti
         assertEquals( 1, messageCaptor.getAllValues().size() );
 
         String loggedMessage = messageCaptor.getValue();
-        assertThat( loggedMessage, startsWith( "Neo4j Session object leaked" ) );
+        assertThat( loggedMessage, containsString( "Neo4j Session object leaked" ) );
         assertThat( loggedMessage, containsString( "Session was create at" ) );
         assertThat( loggedMessage, containsString(
                 getClass().getSimpleName() + "." + testName.getMethodName() )