From 94f3c07e778ba21e97798aded0bd3f8d13a063e2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Pereira?= <jop@di.uminho.pt>
Date: Sun, 12 Jul 2015 15:57:50 +0100
Subject: [PATCH 1/3] Use ObjectWriter from AbstractSocketAppender in
 AbstractSenderSocketAppender.

This provides the same mechanism for overriding the serialization method
in both socket-based remote appenders.
---
 .../logback/core/net/AbstractSocketAppender.java   |  4 +--
 .../logback/core/net/AutoFlushingObjectWriter.java |  7 +++++
 .../java/ch/qos/logback/core/net/ObjectWriter.java |  3 +-
 .../qos/logback/core/net/ObjectWriterFactory.java  |  2 +-
 .../net/server/AbstractServerSocketAppender.java   |  7 ++++-
 .../net/server/RemoteReceiverServerListener.java   | 11 +++++--
 .../net/server/RemoteReceiverStreamClient.java     | 34 ++++++++++------------
 .../net/AbstractSocketAppenderIntegrationTest.java |  6 ++--
 .../core/net/AbstractSocketAppenderTest.java       | 10 +++----
 .../InstrumentedServerSocketAppenderBase.java      |  3 +-
 .../net/server/RemoteReceiverStreamClientTest.java |  3 +-
 11 files changed, 54 insertions(+), 36 deletions(-)

diff --git a/logback-core/src/main/java/ch/qos/logback/core/net/AbstractSocketAppender.java b/logback-core/src/main/java/ch/qos/logback/core/net/AbstractSocketAppender.java
index 229f691..ec9b61a 100755
--- a/logback-core/src/main/java/ch/qos/logback/core/net/AbstractSocketAppender.java
+++ b/logback-core/src/main/java/ch/qos/logback/core/net/AbstractSocketAppender.java
@@ -100,7 +100,7 @@ public abstract class AbstractSocketAppender<E> extends AppenderBase<E>
   /**
    * Constructs a new appender using the given {@link QueueFactory} and {@link ObjectWriterFactory}.
    */
-  AbstractSocketAppender(QueueFactory queueFactory, ObjectWriterFactory objectWriterFactory) {
+  protected AbstractSocketAppender(QueueFactory queueFactory, ObjectWriterFactory objectWriterFactory) {
     this.objectWriterFactory = objectWriterFactory;
     this.queueFactory = queueFactory;
   }
@@ -212,7 +212,7 @@ public abstract class AbstractSocketAppender<E> extends AppenderBase<E>
 
   private ObjectWriter createObjectWriterForSocket() throws IOException {
     socket.setSoTimeout(acceptConnectionTimeout);
-    ObjectWriter objectWriter = objectWriterFactory.newAutoFlushingObjectWriter(socket.getOutputStream());
+    ObjectWriter objectWriter = objectWriterFactory.newObjectWriter(socket.getOutputStream());
     socket.setSoTimeout(0);
     return objectWriter;
   }
diff --git a/logback-core/src/main/java/ch/qos/logback/core/net/AutoFlushingObjectWriter.java b/logback-core/src/main/java/ch/qos/logback/core/net/AutoFlushingObjectWriter.java
index 2298075..2f504d6 100644
--- a/logback-core/src/main/java/ch/qos/logback/core/net/AutoFlushingObjectWriter.java
+++ b/logback-core/src/main/java/ch/qos/logback/core/net/AutoFlushingObjectWriter.java
@@ -57,4 +57,11 @@ public class AutoFlushingObjectWriter implements ObjectWriter {
       writeCounter = 0;
     }
   }
+  
+  /**
+   * {@inheritDoc}
+   */
+  public void close() throws IOException {
+	  objectOutputStream.close();
+  }
 }
diff --git a/logback-core/src/main/java/ch/qos/logback/core/net/ObjectWriter.java b/logback-core/src/main/java/ch/qos/logback/core/net/ObjectWriter.java
index 6682e76..e3e4667 100644
--- a/logback-core/src/main/java/ch/qos/logback/core/net/ObjectWriter.java
+++ b/logback-core/src/main/java/ch/qos/logback/core/net/ObjectWriter.java
@@ -13,6 +13,7 @@
  */
 package ch.qos.logback.core.net;
 
+import java.io.Closeable;
 import java.io.IOException;
 
 /**
@@ -20,7 +21,7 @@ import java.io.IOException;
  *
  * @author Sebastian Gr&ouml;bler
  */
-public interface ObjectWriter {
+public interface ObjectWriter extends Closeable {
 
   /**
    * Writes an object to an output.
diff --git a/logback-core/src/main/java/ch/qos/logback/core/net/ObjectWriterFactory.java b/logback-core/src/main/java/ch/qos/logback/core/net/ObjectWriterFactory.java
index 0bc7ccc..60f65ae 100644
--- a/logback-core/src/main/java/ch/qos/logback/core/net/ObjectWriterFactory.java
+++ b/logback-core/src/main/java/ch/qos/logback/core/net/ObjectWriterFactory.java
@@ -33,7 +33,7 @@ public class ObjectWriterFactory {
    * @return a new {@link ch.qos.logback.core.net.AutoFlushingObjectWriter} instance
    * @throws IOException if an I/O error occurs while writing stream header
    */
-  public AutoFlushingObjectWriter newAutoFlushingObjectWriter(OutputStream outputStream) throws IOException {
+  public ObjectWriter newObjectWriter(OutputStream outputStream) throws IOException {
     return new AutoFlushingObjectWriter(new ObjectOutputStream(outputStream), CoreConstants.OOS_RESET_FREQUENCY);
   }
 }
diff --git a/logback-core/src/main/java/ch/qos/logback/core/net/server/AbstractServerSocketAppender.java b/logback-core/src/main/java/ch/qos/logback/core/net/server/AbstractServerSocketAppender.java
index 57a98f1..81a0635 100644
--- a/logback-core/src/main/java/ch/qos/logback/core/net/server/AbstractServerSocketAppender.java
+++ b/logback-core/src/main/java/ch/qos/logback/core/net/server/AbstractServerSocketAppender.java
@@ -24,6 +24,7 @@ import javax.net.ServerSocketFactory;
 
 import ch.qos.logback.core.AppenderBase;
 import ch.qos.logback.core.net.AbstractSocketAppender;
+import ch.qos.logback.core.net.ObjectWriterFactory;
 import ch.qos.logback.core.spi.PreSerializationTransformer;
 
 /**
@@ -72,7 +73,7 @@ public abstract class AbstractServerSocketAppender<E> extends AppenderBase<E> {
   
   protected ServerListener<RemoteReceiverClient> createServerListener(
       ServerSocket socket) {
-    return new RemoteReceiverServerListener(socket);
+    return new RemoteReceiverServerListener(socket, createObjectWriterFactory());
   }
   
   protected ServerRunner<RemoteReceiverClient> createServerRunner(
@@ -82,6 +83,10 @@ public abstract class AbstractServerSocketAppender<E> extends AppenderBase<E> {
         getClientQueueSize());
   }
   
+  protected ObjectWriterFactory createObjectWriterFactory() {
+	  return new ObjectWriterFactory();
+  }
+  
   @Override
   public void stop() {
     if (!isStarted()) return;
diff --git a/logback-core/src/main/java/ch/qos/logback/core/net/server/RemoteReceiverServerListener.java b/logback-core/src/main/java/ch/qos/logback/core/net/server/RemoteReceiverServerListener.java
index f6ad600..861ee09 100644
--- a/logback-core/src/main/java/ch/qos/logback/core/net/server/RemoteReceiverServerListener.java
+++ b/logback-core/src/main/java/ch/qos/logback/core/net/server/RemoteReceiverServerListener.java
@@ -17,6 +17,8 @@ import java.io.IOException;
 import java.net.ServerSocket;
 import java.net.Socket;
 
+import ch.qos.logback.core.net.ObjectWriterFactory;
+
 /**
  * A {@link ServerListener} that accepts connections from remote receiver
  * component clients.
@@ -26,13 +28,16 @@ import java.net.Socket;
 class RemoteReceiverServerListener
     extends ServerSocketListener<RemoteReceiverClient> {
 
-  /**
+  private ObjectWriterFactory objectWriterFactory;
+
+/**
    * Constructs a new listener.
    * @param serverSocket server socket from which new client connections
    *    will be accepted
    */
-  public RemoteReceiverServerListener(ServerSocket serverSocket) {
+  public RemoteReceiverServerListener(ServerSocket serverSocket, ObjectWriterFactory objectWriterFactory) {
     super(serverSocket);
+    this.objectWriterFactory = objectWriterFactory;
   }
 
   /**
@@ -41,7 +46,7 @@ class RemoteReceiverServerListener
   @Override
   protected RemoteReceiverClient createClient(String id, Socket socket)
       throws IOException {
-    return new RemoteReceiverStreamClient(id, socket);
+    return new RemoteReceiverStreamClient(id, socket, objectWriterFactory);
   }
 
 }
diff --git a/logback-core/src/main/java/ch/qos/logback/core/net/server/RemoteReceiverStreamClient.java b/logback-core/src/main/java/ch/qos/logback/core/net/server/RemoteReceiverStreamClient.java
index a6a1f91..ce2224e 100644
--- a/logback-core/src/main/java/ch/qos/logback/core/net/server/RemoteReceiverStreamClient.java
+++ b/logback-core/src/main/java/ch/qos/logback/core/net/server/RemoteReceiverStreamClient.java
@@ -22,6 +22,8 @@ import java.net.SocketException;
 import java.util.concurrent.BlockingQueue;
 
 import ch.qos.logback.core.CoreConstants;
+import ch.qos.logback.core.net.ObjectWriter;
+import ch.qos.logback.core.net.ObjectWriterFactory;
 import ch.qos.logback.core.spi.ContextAwareBase;
 import ch.qos.logback.core.util.CloseUtil;
 
@@ -40,15 +42,18 @@ class RemoteReceiverStreamClient
   
   private BlockingQueue<Serializable> queue;
   
+  private ObjectWriterFactory objectWriterFactory;
+  
   /**
    * Constructs a new client.
    * @param id identifier string for the client
    * @param socket socket to which logging events will be written
    */
-  public RemoteReceiverStreamClient(String id, Socket socket) {
+  public RemoteReceiverStreamClient(String id, Socket socket, ObjectWriterFactory objectWriterFactory) {
     this.clientId = "client " + id + ": ";
     this.socket = socket;
     this.outputStream = null;
+    this.objectWriterFactory = objectWriterFactory;
   }
 
   /**
@@ -60,10 +65,11 @@ class RemoteReceiverStreamClient
    * @param id identifier string for the client
    * @param outputStream output stream to which logging Events will be written
    */
-  RemoteReceiverStreamClient(String id, OutputStream outputStream) {
+  RemoteReceiverStreamClient(String id, OutputStream outputStream, ObjectWriterFactory objectWriterFactory) {
     this.clientId = "client " + id + ": ";
     this.socket = null;
     this.outputStream = outputStream;
+    this.objectWriterFactory = objectWriterFactory;
   }
   
   /**
@@ -97,21 +103,13 @@ class RemoteReceiverStreamClient
   public void run() {  
     addInfo(clientId + "connected"); 
 
-    ObjectOutputStream oos = null;
+    ObjectWriter ow = null;
     try {
-      int counter = 0;
-      oos = createObjectOutputStream();
+      ow = createObjectWriter();
       while (!Thread.currentThread().isInterrupted()) {
         try {
           Serializable event = queue.take();
-          oos.writeObject(event);
-          oos.flush();
-          if (++counter >= CoreConstants.OOS_RESET_FREQUENCY) {
-            // failing to reset the stream periodically will result in a
-            // serious memory leak (as noted in AbstractSocketAppender)
-            counter = 0;
-            oos.reset();
-          }
+          ow.write(event);
         }
         catch (InterruptedException ex) {
           Thread.currentThread().interrupt();
@@ -128,18 +126,18 @@ class RemoteReceiverStreamClient
       addError(clientId + ex);
     }
     finally {
-      if (oos != null) {
-        CloseUtil.closeQuietly(oos);
+      if (ow != null) {
+        CloseUtil.closeQuietly(ow);
       }
       close();
       addInfo(clientId + "connection closed");
     }
   }
 
-  private ObjectOutputStream createObjectOutputStream() throws IOException {
+  private ObjectWriter createObjectWriter() throws IOException {
     if (socket == null) {
-      return new ObjectOutputStream(outputStream);
+      return objectWriterFactory.newObjectWriter(outputStream);
     }
-    return new ObjectOutputStream(socket.getOutputStream());
+    return objectWriterFactory.newObjectWriter(socket.getOutputStream());
   }
 }
diff --git a/logback-core/src/test/java/ch/qos/logback/core/net/AbstractSocketAppenderIntegrationTest.java b/logback-core/src/test/java/ch/qos/logback/core/net/AbstractSocketAppenderIntegrationTest.java
index dfea435..5eccb50 100755
--- a/logback-core/src/test/java/ch/qos/logback/core/net/AbstractSocketAppenderIntegrationTest.java
+++ b/logback-core/src/test/java/ch/qos/logback/core/net/AbstractSocketAppenderIntegrationTest.java
@@ -52,7 +52,7 @@ public class AbstractSocketAppenderIntegrationTest {
 
   private ThreadPoolExecutor executorService = (ThreadPoolExecutor) Executors.newCachedThreadPool();
   private MockContext mockContext = new MockContext(executorService);
-  private AutoFlushingObjectWriter objectWriter;
+  private ObjectWriter objectWriter;
   private ObjectWriterFactory objectWriterFactory = new SpyProducingObjectWriterFactory();
   private LinkedBlockingDeque<String> deque = spy(new LinkedBlockingDeque<String>(1));
   private QueueFactory queueFactory = mock(QueueFactory.class);
@@ -120,8 +120,8 @@ public class AbstractSocketAppenderIntegrationTest {
   private class SpyProducingObjectWriterFactory extends ObjectWriterFactory {
 
     @Override
-    public AutoFlushingObjectWriter newAutoFlushingObjectWriter(OutputStream outputStream) throws IOException {
-      objectWriter = spy(super.newAutoFlushingObjectWriter(outputStream));
+    public ObjectWriter newObjectWriter(OutputStream outputStream) throws IOException {
+      objectWriter = spy(super.newObjectWriter(outputStream));
       return objectWriter;
     }
   }
diff --git a/logback-core/src/test/java/ch/qos/logback/core/net/AbstractSocketAppenderTest.java b/logback-core/src/test/java/ch/qos/logback/core/net/AbstractSocketAppenderTest.java
index eb04125..94315db 100755
--- a/logback-core/src/test/java/ch/qos/logback/core/net/AbstractSocketAppenderTest.java
+++ b/logback-core/src/test/java/ch/qos/logback/core/net/AbstractSocketAppenderTest.java
@@ -77,7 +77,7 @@ public class AbstractSocketAppenderTest {
   @Before
   public void setupValidAppenderWithMockDependencies() throws Exception {
 
-    doReturn(objectWriter).when(objectWriterFactory).newAutoFlushingObjectWriter(any(OutputStream.class));
+    doReturn(objectWriter).when(objectWriterFactory).newObjectWriter(any(OutputStream.class));
     doReturn(deque).when(queueFactory).<String>newLinkedBlockingDeque(anyInt());
 
     appender.setContext(mockContext);
@@ -204,7 +204,7 @@ public class AbstractSocketAppenderTest {
 
     // given
     mockOneSuccessfulSocketConnection();
-    doThrow(new IOException()).when(objectWriterFactory).newAutoFlushingObjectWriter(any(OutputStream.class));
+    doThrow(new IOException()).when(objectWriterFactory).newObjectWriter(any(OutputStream.class));
     appender.start();
 
     // when
@@ -219,7 +219,7 @@ public class AbstractSocketAppenderTest {
 
     // given
     mockOneSuccessfulSocketConnection();
-    doThrow(new IOException()).when(objectWriterFactory).newAutoFlushingObjectWriter(any(OutputStream.class));
+    doThrow(new IOException()).when(objectWriterFactory).newObjectWriter(any(OutputStream.class));
     appender.start();
 
     // when
@@ -234,7 +234,7 @@ public class AbstractSocketAppenderTest {
 
     // given
     mockOneSuccessfulSocketConnection();
-    doThrow(new IOException()).when(objectWriterFactory).newAutoFlushingObjectWriter(any(OutputStream.class));
+    doThrow(new IOException()).when(objectWriterFactory).newObjectWriter(any(OutputStream.class));
     appender.start();
 
     // when
@@ -438,7 +438,7 @@ public class AbstractSocketAppenderTest {
     appender.append("some event");
 
     // then
-    verify(objectWriterFactory, timeout(TIMEOUT).atLeast(2)).newAutoFlushingObjectWriter(any(OutputStream.class));
+    verify(objectWriterFactory, timeout(TIMEOUT).atLeast(2)).newObjectWriter(any(OutputStream.class));
   }
 
   @Test
diff --git a/logback-core/src/test/java/ch/qos/logback/core/net/server/InstrumentedServerSocketAppenderBase.java b/logback-core/src/test/java/ch/qos/logback/core/net/server/InstrumentedServerSocketAppenderBase.java
index 5828b53..170273d 100644
--- a/logback-core/src/test/java/ch/qos/logback/core/net/server/InstrumentedServerSocketAppenderBase.java
+++ b/logback-core/src/test/java/ch/qos/logback/core/net/server/InstrumentedServerSocketAppenderBase.java
@@ -21,6 +21,7 @@ import java.util.concurrent.Executor;
 
 import javax.net.ServerSocketFactory;
 
+import ch.qos.logback.core.net.ObjectWriterFactory;
 import ch.qos.logback.core.spi.PreSerializationTransformer;
 
 /**
@@ -38,7 +39,7 @@ public class InstrumentedServerSocketAppenderBase
   private ServerListener lastListener;
   
   public InstrumentedServerSocketAppenderBase(ServerSocket serverSocket) {
-    this(serverSocket, new RemoteReceiverServerListener(serverSocket), null);
+    this(serverSocket, new RemoteReceiverServerListener(serverSocket, new ObjectWriterFactory()), null);
   }
   
   public InstrumentedServerSocketAppenderBase(ServerSocket serverSocket,
diff --git a/logback-core/src/test/java/ch/qos/logback/core/net/server/RemoteReceiverStreamClientTest.java b/logback-core/src/test/java/ch/qos/logback/core/net/server/RemoteReceiverStreamClientTest.java
index 7932486..de15d4d 100644
--- a/logback-core/src/test/java/ch/qos/logback/core/net/server/RemoteReceiverStreamClientTest.java
+++ b/logback-core/src/test/java/ch/qos/logback/core/net/server/RemoteReceiverStreamClientTest.java
@@ -23,6 +23,7 @@ import java.io.ObjectInputStream;
 import org.junit.Before;
 import org.junit.Test;
 
+import ch.qos.logback.core.net.ObjectWriterFactory;
 import ch.qos.logback.core.net.mock.MockContext;
 
 
@@ -43,7 +44,7 @@ public class RemoteReceiverStreamClientTest {
       new ByteArrayOutputStream();
   
   private RemoteReceiverStreamClient client = 
-      new RemoteReceiverStreamClient("someId", outputStream);
+      new RemoteReceiverStreamClient("someId", outputStream, new ObjectWriterFactory());
   
   @Before
   public void setUp() throws Exception {
-- 
2.5.0

