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]); + } } } }
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.
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.
This also affects trunk.
Fixed in SVN trunk. Also added unit test to try to reproduce leaks.
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.
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.