logging-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rgo...@apache.org
Subject svn commit: r1470110 - in /logging/log4j/log4j2/trunk: core/src/main/java/org/apache/logging/log4j/core/appender/ core/src/main/java/org/apache/logging/log4j/core/net/ core/src/test/java/org/apache/logging/log4j/core/appender/ core/src/test/java/org/ap...
Date Sat, 20 Apr 2013 05:58:23 GMT
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



Mime
View raw message