Bug 37282

Summary: SyslogAppender leaks descriptors
Product: Log4j Reporter: Ian Reilly <iwreilly>
Component: AppenderAssignee: log4j-dev <log4j-dev>
Status: RESOLVED FIXED    
Severity: normal CC: kay.abendroth
Priority: P2    
Version: 1.2   
Target Milestone: ---   
Hardware: Other   
OS: Linux   
Bug Depends on:    
Bug Blocks: 40951    

Description Ian Reilly 2005-10-27 22:05:32 UTC
The attached patch for version 1.2.12 fixes two problems in 
org.apache.log4j.net.SyslogAppender.  First, it doesn't close the writer and as 
a result leaks UDP sockets.  Second, when logging an exception, it assumes that 
all lines after the first line start with a \t character.  If this is not the 
case, it removes the first character from the line (or throws an exception if 
the line is empty).

--- SyslogAppender.java.orig    2005-10-25 15:50:35.000000000 -0400
+++ SyslogAppender.java 2005-10-25 15:19:57.000000000 -0400
@@ -121,8 +121,11 @@
  public
  void close() {
    closed = true; 
-    // A SyslogWriter is UDP based and needs no opening. Hence, it
-    // can't be closed. We just unset the variables here.
+    try {
+       if (sqw != null) sqw.close();
+    } catch (Exception e) {
+       // ignore error
+    }
    sqw = null;
  }

@@ -260,7 +263,15 @@
        if(len > 0) {
          sqw.write(s[0]);
          for(int i = 1; i < len; i++) {
-            sqw.write (TAB+s[i].substring(1));
+            if (s[i].length() > 0) {
+               if (s[i].charAt(0) == '\t') {
+                  sqw.write(TAB+s[i].substring(1));
+               } else {
+                   sqw.write(TAB+s[i]);
+               }
+            } else {
+               sqw.write(s[i]);
+            }
          }
        }
      }
Comment 1 Curt Arnold 2006-08-30 21:49:50 UTC
Could you review your patch against the current SVN for log4j 1.2 and confirm that it actually eliminates 
the descriptor leakage?  As far as I can see, the sqw.close() statement would have no significant effect 
since org.apache.log4j.helpers.SyslogWriter.close() is a no-op.

If you do resubmit a patch, spin the \t patch into a second bug report and patch.

Comment 2 Ian Reilly 2006-09-13 20:13:08 UTC
I forgot to include part of my patch.  SyslogWriter must be patched to 
implement the close method.  The following patch will do that:

*** SyslogWriter.java.orig	2006-09-13 16:11:07.000000000 -0400
--- SyslogWriter.java	2006-09-13 16:11:21.000000000 -0400
***************
*** 82,86 ****
    void flush() {}
  
    public
!   void close() {}
  }
--- 82,86 ----
    void flush() {}
  
    public
!   void close() {if (ds != null) ds.close();}
  }


I have opened a separate bug report for the tab issue.
Comment 3 Kay Abendroth 2006-11-13 04:14:54 UTC
This also affects trunk.
Comment 4 Elias Ross 2007-01-26 21:29:03 UTC
Fixed in SVN trunk.  Also added unit test to try to reproduce leaks.
Comment 5 Curt Arnold 2007-01-30 13:59:16 UTC
Elias committed changes against log4j/trunk in rev 500483.  Not sure that tests are actually run or if they 
would fail if the changes if the changes were not made.

Reopening since for review of the trunk changes and careful consideration on backporting to log4j 1.2.
Comment 6 Curt Arnold 2007-02-21 14:46:24 UTC
Tab bug is 40502.

Rev 50048 on log4j trunk addresses multiple issues, not just leaks.  Should br reverted and then broken 
down into discrete changes.

Committed rev 510279 on 1.2 branch which just addresses close() issue and adds testActualLogging unit 
test.