Bug 37282 - SyslogAppender leaks descriptors
SyslogAppender leaks descriptors
Status: RESOLVED FIXED
Product: Log4j
Classification: Unclassified
Component: Appender
1.2
Other Linux
: P2 normal
: ---
Assigned To: log4j-dev
:
Depends on:
Blocks: 40951
  Show dependency tree
 
Reported: 2005-10-27 22:05 UTC by Ian Reilly
Modified: 2007-02-22 13:51 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
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.