Author: rgoers
Date: Sat Apr 20 05:58:22 2013
New Revision: 1470110

URL: http://svn.apache.org/r1470110
Log:
LOG4J2-205 - Fix deadlock in SocketAppender. Added option to not wait for 
socket reconnect.

Modified:
    
logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/appender/SocketAppender.java
    
logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/appender/SyslogAppender.java
    
logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/net/TCPSocketManager.java
    
logging/log4j/log4j2/trunk/core/src/test/java/org/apache/logging/log4j/core/appender/SocketAppenderTest.java
    
logging/log4j/log4j2/trunk/core/src/test/java/org/apache/logging/log4j/core/appender/SyslogAppenderTest.java
    
logging/log4j/log4j2/trunk/core/src/test/java/org/apache/logging/log4j/core/net/SocketServerTest.java
    logging/log4j/log4j2/trunk/src/changes/changes.xml
    logging/log4j/log4j2/trunk/src/site/xdoc/manual/appenders.xml

Modified: 
logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/appender/SocketAppender.java
URL: 
http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/appender/SocketAppender.java?rev=1470110&r1=1470109&r2=1470110&view=diff
==============================================================================
--- 
logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/appender/SocketAppender.java
 (original)
+++ 
logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/appender/SocketAppender.java
 Sat Apr 20 05:58:22 2013
@@ -71,6 +71,7 @@ public class SocketAppender extends Abst
      * @param portNum The port to connect to on the target host.
      * @param protocol The Protocol to use.
      * @param delay The interval in which failed writes should be retried.
+     * @param immediateFail True if the write should fail if no socket is 
immediately available.
      * @param name The name of the Appender.
      * @param immediateFlush "true" if data should be flushed on each write.
      * @param suppress "true" if exceptions should be hidden from the 
application, "false" otherwise.
@@ -78,7 +79,7 @@ public class SocketAppender extends Abst
      * @param layout The layout to use (defaults to SerializedLayout).
      * @param filter The Filter or null.
      * @param advertise "true" if the appender configuration should be 
advertised, "false" otherwise.
-     * @param config The Configuration               
+     * @param config The Configuration
      * @return A SocketAppender.
      */
     @PluginFactory
@@ -86,6 +87,7 @@ public class SocketAppender extends Abst
                                                 @PluginAttr("port") final 
String portNum,
                                                 @PluginAttr("protocol") final 
String protocol,
                                                 
@PluginAttr("reconnectionDelay") final String delay,
+                                                @PluginAttr("immediateFail") 
final String immediateFail,
                                                 @PluginAttr("name") final 
String name,
                                                 @PluginAttr("immediateFlush") 
final String immediateFlush,
                                                 
@PluginAttr("suppressExceptions") final String suppress,
@@ -97,6 +99,7 @@ public class SocketAppender extends Abst
         final boolean isFlush = immediateFlush == null ? true : 
Boolean.valueOf(immediateFlush);
         boolean isAdvertise = advertise == null ? false : 
Boolean.valueOf(advertise);
         final boolean handleExceptions = suppress == null ? true : 
Boolean.valueOf(suppress);
+        final boolean fail = immediateFail == null ? true : 
Boolean.valueOf(immediateFail);
         final int reconnectDelay = delay == null ? 0 : Integer.parseInt(delay);
         final int port = portNum == null ? 0 : Integer.parseInt(portNum);
         if (layout == null) {
@@ -110,7 +113,7 @@ public class SocketAppender extends Abst
 
         final String prot = protocol != null ? protocol : Protocol.TCP.name();
 
-        final AbstractSocketManager manager = createSocketManager(prot, host, 
port, reconnectDelay);
+        final AbstractSocketManager manager = createSocketManager(prot, host, 
port, reconnectDelay, fail);
         if (manager == null) {
             return null;
         }
@@ -119,11 +122,11 @@ public class SocketAppender extends Abst
     }
 
     protected static AbstractSocketManager createSocketManager(final String 
protocol, final String host, final int port,
-                                                               final int 
delay) {
+                                                               final int 
delay, final boolean immediateFail) {
         final Protocol p = EnglishEnums.valueOf(Protocol.class, protocol);
         switch (p) {
             case TCP:
-                return TCPSocketManager.getSocketManager(host, port, delay);
+                return TCPSocketManager.getSocketManager(host, port, delay, 
immediateFail);
             case UDP:
                 return DatagramSocketManager.getSocketManager(host, port);
             default:

Modified: 
logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/appender/SyslogAppender.java
URL: 
http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/appender/SyslogAppender.java?rev=1470110&r1=1470109&r2=1470110&view=diff
==============================================================================
--- 
logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/appender/SyslogAppender.java
 (original)
+++ 
logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/appender/SyslogAppender.java
 Sat Apr 20 05:58:22 2013
@@ -52,6 +52,7 @@ public class SyslogAppender extends Sock
      * @param portNum The port to connect to on the target host.
      * @param protocol The Protocol to use.
      * @param delay The interval in which failed writes should be retried.
+     * @param immediateFail True if the write should fail if no socket is 
immediately available.
      * @param name The name of the Appender.
      * @param immediateFlush "true" if data should be flushed on each write.
      * @param suppress "true" if exceptions should be hidden from the 
application, "false" otherwise.
@@ -84,6 +85,7 @@ public class SyslogAppender extends Sock
                                                 @PluginAttr("port") final 
String portNum,
                                                 @PluginAttr("protocol") final 
String protocol,
                                                 
@PluginAttr("reconnectionDelay") final String delay,
+                                                @PluginAttr("immediateFail") 
final String immediateFail,
                                                 @PluginAttr("name") final 
String name,
                                                 @PluginAttr("immediateFlush") 
final String immediateFlush,
                                                 
@PluginAttr("suppressExceptions") final String suppress,
@@ -111,6 +113,7 @@ public class SyslogAppender extends Sock
         final boolean isFlush = immediateFlush == null ? true : 
Boolean.valueOf(immediateFlush);
         final boolean handleExceptions = suppress == null ? true : 
Boolean.valueOf(suppress);
         final int reconnectDelay = delay == null ? 0 : Integer.parseInt(delay);
+        final boolean fail = immediateFail == null ? true : 
Boolean.valueOf(immediateFail);
         final int port = portNum == null ? 0 : Integer.parseInt(portNum);
         boolean isAdvertise = advertise == null ? false : 
Boolean.valueOf(advertise);
         final Layout<String> layout = RFC5424.equalsIgnoreCase(format) ?
@@ -123,7 +126,7 @@ public class SyslogAppender extends Sock
             return null;
         }
         final String prot = protocol != null ? protocol : Protocol.UDP.name();
-        final AbstractSocketManager manager = createSocketManager(prot, host, 
port, reconnectDelay);
+        final AbstractSocketManager manager = createSocketManager(prot, host, 
port, reconnectDelay, fail);
         if (manager == null) {
             return null;
         }

Modified: 
logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/net/TCPSocketManager.java
URL: 
http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/net/TCPSocketManager.java?rev=1470110&r1=1470109&r2=1470110&view=diff
==============================================================================
--- 
logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/net/TCPSocketManager.java
 (original)
+++ 
logging/log4j/log4j2/trunk/core/src/main/java/org/apache/logging/log4j/core/net/TCPSocketManager.java
 Sat Apr 20 05:58:22 2013
@@ -54,6 +54,8 @@ public class TCPSocketManager extends Ab
 
     private final boolean retry;
 
+    private final boolean immediateFail;
+
     /**
      * The Constructor.
      * @param name The unique name of this connection.
@@ -65,10 +67,11 @@ public class TCPSocketManager extends Ab
      * @param delay Reconnection interval.
      */
     public TCPSocketManager(final String name, final OutputStream os, final 
Socket sock, final InetAddress addr,
-                            final String host, final int port, final int 
delay) {
+                            final String host, final int port, final int 
delay, final boolean immediateFail) {
         super(name, os, addr, host, port);
         this.reconnectionDelay = delay;
         this.socket = sock;
+        this.immediateFail = immediateFail;
         retry = delay > 0;
         if (sock == null) {
             connector = new Reconnector(this);
@@ -85,7 +88,7 @@ public class TCPSocketManager extends Ab
      * @param delay The interval to pause between retries.
      * @return A TCPSocketManager.
      */
-    public static TCPSocketManager getSocketManager(final String host, int 
port, int delay) {
+    public static TCPSocketManager getSocketManager(final String host, int 
port, int delay, boolean immediateFail) {
         if (host == null || host.length() == 0) {
             throw new IllegalArgumentException("A host name is required");
         }
@@ -95,13 +98,14 @@ public class TCPSocketManager extends Ab
         if (delay == 0) {
             delay = DEFAULT_RECONNECTION_DELAY;
         }
-        return (TCPSocketManager) getManager("TCP:" + host + ":" + port, new 
FactoryData(host, port, delay), FACTORY);
+        return (TCPSocketManager) getManager("TCP:" + host + ":" + port,
+            new FactoryData(host, port, delay, immediateFail), FACTORY);
     }
 
     @Override
-    protected synchronized void write(final byte[] bytes, final int offset, 
final int length)  {
+    protected void write(final byte[] bytes, final int offset, final int 
length)  {
         if (socket == null) {
-            if (connector != null) {
+            if (connector != null && !immediateFail) {
                 connector.latch();
             }
             if (socket == null) {
@@ -109,17 +113,19 @@ public class TCPSocketManager extends Ab
                 throw new AppenderRuntimeException(msg);
             }
         }
-        try {
-            getOutputStream().write(bytes, offset, length);
-        } catch (final IOException ex) {
-            if (retry && connector == null) {
-                connector = new Reconnector(this);
-                connector.setDaemon(true);
-                connector.setPriority(Thread.MIN_PRIORITY);
-                connector.start();
+        synchronized (this) {
+            try {
+                getOutputStream().write(bytes, offset, length);
+            } catch (final IOException ex) {
+                if (retry && connector == null) {
+                    connector = new Reconnector(this);
+                    connector.setDaemon(true);
+                    connector.setPriority(Thread.MIN_PRIORITY);
+                    connector.start();
+                }
+                final String msg = "Error writing to " + getName();
+                throw new AppenderRuntimeException(msg, ex);
             }
-            final String msg = "Error writing to " + getName();
-            throw new AppenderRuntimeException(msg, ex);
         }
     }
 
@@ -146,7 +152,7 @@ public class TCPSocketManager extends Ab
         result.put("direction", "out");
         return result;
     }
-    
+
     /**
      * Handles reconnecting to a Thread.
      */
@@ -214,11 +220,13 @@ public class TCPSocketManager extends Ab
         private final String host;
         private final int port;
         private final int delay;
+        private final boolean immediateFail;
 
-        public FactoryData(final String host, final int port, final int delay) 
{
+        public FactoryData(final String host, final int port, final int delay, 
final boolean immediateFail) {
             this.host = host;
             this.port = port;
             this.delay = delay;
+            this.immediateFail = immediateFail;
         }
     }
 
@@ -240,7 +248,8 @@ public class TCPSocketManager extends Ab
             try {
                 final Socket socket = new Socket(data.host, data.port);
                 os = socket.getOutputStream();
-                return new TCPSocketManager(name, os, socket, address, 
data.host, data.port, data.delay);
+                return new TCPSocketManager(name, os, socket, address, 
data.host, data.port, data.delay,
+                    data.immediateFail);
             } catch (final IOException ex) {
                 LOGGER.error("TCPSocketManager (" + name + ") " + ex);
                 os = new ByteArrayOutputStream();
@@ -248,7 +257,7 @@ public class TCPSocketManager extends Ab
             if (data.delay == 0) {
                 return null;
             }
-            return new TCPSocketManager(name, os, null, address, data.host, 
data.port, data.delay);
+            return new TCPSocketManager(name, os, null, address, data.host, 
data.port, data.delay, data.immediateFail);
         }
     }
 }

Modified: 
logging/log4j/log4j2/trunk/core/src/test/java/org/apache/logging/log4j/core/appender/SocketAppenderTest.java
URL: 
http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/core/src/test/java/org/apache/logging/log4j/core/appender/SocketAppenderTest.java?rev=1470110&r1=1470109&r2=1470110&view=diff
==============================================================================
--- 
logging/log4j/log4j2/trunk/core/src/test/java/org/apache/logging/log4j/core/appender/SocketAppenderTest.java
 (original)
+++ 
logging/log4j/log4j2/trunk/core/src/test/java/org/apache/logging/log4j/core/appender/SocketAppenderTest.java
 Sat Apr 20 05:58:22 2013
@@ -43,6 +43,7 @@ import java.util.concurrent.TimeUnit;
 
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
 
 /**
  *
@@ -51,7 +52,10 @@ public class SocketAppenderTest {
 
     private static final String HOST = "localhost";
     private static final String PORT = "8199";
-    private static final int PORTNUM = Integer.parseInt(PORT);
+    private static final String DYN_PORT = "8300";
+    private static final String ERROR_PORT = "8301";
+    private static final int PORTNUM1 = Integer.parseInt(PORT);
+    private static final int PORTNUM2 = Integer.parseInt(DYN_PORT);
 
     private static BlockingQueue<LogEvent> list = new 
ArrayBlockingQueue<LogEvent>(10);
 
@@ -66,7 +70,7 @@ public class SocketAppenderTest {
 
     @BeforeClass
     public static void setupClass() throws Exception {
-        tcp = new TCPSocketServer();
+        tcp = new TCPSocketServer(PORTNUM1);
         tcp.start();
         udp = new UDPSocketServer();
         udp.start();
@@ -97,7 +101,7 @@ public class SocketAppenderTest {
     public void testTCPAppender() throws Exception {
 
         final SocketAppender appender = 
SocketAppender.createAppender("localhost", PORT, "tcp", "-1",
-            "Test", null, null, null, null, null, null);
+            "false", "Test", null, null, null, null, null, null);
         appender.start();
 
         // set appender on root and set level to debug
@@ -125,7 +129,7 @@ public class SocketAppenderTest {
     public void testUDPAppender() throws Exception {
 
         final SocketAppender appender = 
SocketAppender.createAppender("localhost", PORT, "udp", "-1",
-            "Test", null, null, null, null, null, null);
+            "false", "Test", null, null, null, null, null, null);
         appender.start();
 
         // set appender on root and set level to debug
@@ -139,13 +143,52 @@ public class SocketAppenderTest {
         assertTrue("Message not delivered via UDP", udpCount > 0);
     }
 
+    @Test
+    public void testTcpAppenderDeadlock() throws Exception {
+
+        final SocketAppender appender = 
SocketAppender.createAppender("localhost", DYN_PORT, "tcp", "10000",
+                "false", "Test", null, null, null, null, null, null);
+            appender.start();
+            // set appender on root and set level to debug
+            root.addAppender(appender);
+            root.setAdditive(false);
+            root.setLevel(Level.DEBUG);
+
+            new TCPSocketServer(PORTNUM2).start();
+
+            root.debug("This message is written because a deadlock never.");
+
+            LogEvent event = list.poll(3, TimeUnit.SECONDS);
+            assertNotNull("No event retrieved", event);
+    }
+
+    @Test
+    public void testTcpAppenderNoWait() throws Exception {
+
+        final SocketAppender appender = 
SocketAppender.createAppender("localhost", ERROR_PORT, "tcp", "10000",
+            "true", "Test", null, "false", null, null, null, null);
+        appender.start();
+        // set appender on root and set level to debug
+        root.addAppender(appender);
+        root.setAdditive(false);
+        root.setLevel(Level.DEBUG);
+
+        try {
+            root.debug("This message is written because a deadlock never.");
+            fail("No Exception was thrown");
+        } catch (Exception ex) {
+            // Failure is expected.
+        }
+    }
+
+
     public static class UDPSocketServer extends Thread {
         private final DatagramSocket sock;
         private boolean shutdown = false;
         private Thread thread;
 
         public UDPSocketServer() throws IOException {
-            this.sock = new DatagramSocket(PORTNUM);
+            this.sock = new DatagramSocket(PORTNUM1);
         }
 
         public void shutdown() {
@@ -178,8 +221,8 @@ public class SocketAppenderTest {
         private final ServerSocket sock;
         private boolean shutdown = false;
 
-        public TCPSocketServer() throws IOException {
-            this.sock = new ServerSocket(PORTNUM);
+        public TCPSocketServer(int port) throws IOException {
+            this.sock = new ServerSocket(port);
         }
 
         public void shutdown() {

Modified: 
logging/log4j/log4j2/trunk/core/src/test/java/org/apache/logging/log4j/core/appender/SyslogAppenderTest.java
URL: 
http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/core/src/test/java/org/apache/logging/log4j/core/appender/SyslogAppenderTest.java?rev=1470110&r1=1470109&r2=1470110&view=diff
==============================================================================
--- 
logging/log4j/log4j2/trunk/core/src/test/java/org/apache/logging/log4j/core/appender/SyslogAppenderTest.java
 (original)
+++ 
logging/log4j/log4j2/trunk/core/src/test/java/org/apache/logging/log4j/core/appender/SyslogAppenderTest.java
 Sat Apr 20 05:58:22 2013
@@ -210,7 +210,7 @@ public class SyslogAppenderTest {
     }
 
     private SyslogAppender createAppender(final String protocol, final String 
format) {
-        return SyslogAppender.createAppender("localhost", PORT, protocol, 
"-1", "Test", "true", "false", "LOCAL0", "Audit",
+        return SyslogAppender.createAppender("localhost", PORT, protocol, 
"-1", null, "Test", "true", "false", "LOCAL0", "Audit",
             "18060", "true", "RequestContext", null, null, "true", null, 
"TestApp", "Test", null, "ipAddress,loginId",
             null, format, null, null, null, null, null);
     }

Modified: 
logging/log4j/log4j2/trunk/core/src/test/java/org/apache/logging/log4j/core/net/SocketServerTest.java
URL: 
http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/core/src/test/java/org/apache/logging/log4j/core/net/SocketServerTest.java?rev=1470110&r1=1470109&r2=1470110&view=diff
==============================================================================
--- 
logging/log4j/log4j2/trunk/core/src/test/java/org/apache/logging/log4j/core/net/SocketServerTest.java
 (original)
+++ 
logging/log4j/log4j2/trunk/core/src/test/java/org/apache/logging/log4j/core/net/SocketServerTest.java
 Sat Apr 20 05:58:22 2013
@@ -92,7 +92,7 @@ public class SocketServerTest {
         final Filter socketFilter = new ThreadFilter(Filter.Result.NEUTRAL, 
Filter.Result.DENY);
         final Filter serverFilter = new ThreadFilter(Filter.Result.DENY, 
Filter.Result.NEUTRAL);
         final SocketAppender appender = 
SocketAppender.createAppender("localhost", PORT, "tcp", "-1",
-            "Test", null, null, null, socketFilter, null, null);
+            null, "Test", null, null, null, socketFilter, null, null);
         appender.start();
         final ListAppender listApp = new ListAppender("Events", serverFilter, 
null, false, false);
         listApp.start();

Modified: logging/log4j/log4j2/trunk/src/changes/changes.xml
URL: 
http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/src/changes/changes.xml?rev=1470110&r1=1470109&r2=1470110&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/src/changes/changes.xml (original)
+++ logging/log4j/log4j2/trunk/src/changes/changes.xml Sat Apr 20 05:58:22 2013
@@ -23,6 +23,9 @@
 
   <body>
     <release version="2.0-beta5" date="@TBD@" description="Bug fixes and 
enhancements">
+      <action issue="LOG4J2-205" dev="rgoers" type="fix">
+        Fix deadlock in SocketAppender. Added option to not wait for socket 
reconnect.
+      </action>
       <action issue="LOG4J2-207" dev="rgoers" type="add" due-to="Remko Popma">
         Add JMX support.
       </action>

Modified: logging/log4j/log4j2/trunk/src/site/xdoc/manual/appenders.xml
URL: 
http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/src/site/xdoc/manual/appenders.xml?rev=1470110&r1=1470109&r2=1470110&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/src/site/xdoc/manual/appenders.xml (original)
+++ logging/log4j/log4j2/trunk/src/site/xdoc/manual/appenders.xml Sat Apr 20 
05:58:22 2013
@@ -1373,7 +1373,7 @@
                                        instead of a
                                        <tt>BufferedOutputStream</tt>.
                                        We saw a 20-200% performance 
improvement compared to
-                                       FileAppender with "bufferedIO=true" in 
our 
+                                       FileAppender with "bufferedIO=true" in 
our
                                        <a 
href="async.html#FastFileAppenderPerformance">measurements</a>.
                                        Similar to the FileAppender,
                                        FastFileAppender uses a FastFileManager 
to actually perform the
@@ -1485,9 +1485,9 @@
                                        <tt>BufferedOutputStream</tt>.
                                        We saw a 20-200% performance 
improvement compared to
                                        RollingFileAppender with 
"bufferedIO=true"
-                                       in our 
+                                       in our
                                        <a 
href="async.html#FastFileAppenderPerformance">measurements</a>.
-                                       
+
                                        The FastRollingFileAppender writes
                                        to the File named in the
                                        fileName parameter
@@ -1496,7 +1496,7 @@
                                        and the RolloverPolicy.
 
                                        Similar to the RollingFileAppender,
-                                       FastRollingFileAppender uses a 
FastRollingFileManager 
+                                       FastRollingFileAppender uses a 
FastRollingFileManager
                                        to actually perform the
                                        file I/O and perform the rollover. 
While FastRollingFileAppender
                                        from different Configurations cannot be
@@ -1569,7 +1569,7 @@
                                                        <a
                                                                
href="http://download.oracle.com/javase/6/docs/api/java/text/SimpleDateFormat.html";>
                                                                
SimpleDateFormat</a>
-                                                       
+
                                                        and/or a %i which 
represents an integer counter. The pattern
                                                        also supports 
interpolation at
                                                        runtime so any of the 
Lookups (such
@@ -1980,6 +1980,12 @@
               <td>The name or address of the system that is listening for log 
events. This parameter is required.</td>
             </tr>
             <tr>
+              <td>immediateFail</td>
+              <td>boolean</td>
+              <td>When set to true, log events will not wait to try to 
reconnect and will fail immediately if the
+              socket is not available.</td>
+            </tr>
+            <tr>
               <td>immediateFlush</td>
               <td>boolean</td>
               <td>When set to true, each write will be followed by a flush. 
This will guarantee the data is written
@@ -2102,6 +2108,12 @@
                 a StructuredDataMessage the id from the Message will be used 
instead of this value.</td>
             </tr>
             <tr>
+              <td>immediateFail</td>
+              <td>boolean</td>
+              <td>When set to true, log events will not wait to try to 
reconnect and will fail immediately if the
+                socket is not available.</td>
+            </tr>
+            <tr>
               <td>immediateFlush</td>
               <td>boolean</td>
               <td>When set to true, each write will be followed by a flush. 
This will guarantee the data is written


Reply via email to