svn commit: r1338278 - in /spamassassin/trunk: lib/Mail/SpamAssassin/Logger/Syslog.pm spamd/spamd.raw

[mm...@apache.org]

Author: mmartinec
Date: Mon May 14 16:31:09 2012
New Revision: 1338278

URL: http://svn.apache.org/viewvc?rev=1338278&view=rev
Log:
Bug 6745: spamd crashes because of memory problems in Logger.pm

Modified:
    spamassassin/trunk/lib/Mail/SpamAssassin/Logger/Syslog.pm
    spamassassin/trunk/spamd/spamd.raw

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Logger/Syslog.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Logger/Syslog.pm?rev=1338278&r1=1338277&r2=1338278&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Logger/Syslog.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Logger/Syslog.pm Mon May 14 16:31:09 2012
@@ -167,17 +167,21 @@ sub log_message {
   }
   $msg = $timestamp . ' ' . $msg  if $timestamp ne '';
 
-  # important: do not call syslog() from the SIGCHLD handler
-  # child_handler().   otherwise we can get into a loop if syslog()
-  # forks a process -- as it does in syslog-ng apparently! (bug 3625)
-  $Mail::SpamAssassin::Logger::LOG_SA{INHIBIT_LOGGING_IN_SIGCHLD_HANDLER} = 1;
+# no longer needed since a patch to bug 6745:
+# # important: do not call syslog() from the SIGCHLD handler
+# # child_handler().   otherwise we can get into a loop if syslog()
+# # forks a process -- as it does in syslog-ng apparently! (bug 3625)
+# $Mail::SpamAssassin::Logger::LOG_SA{INHIBIT_LOGGING_IN_SIGCHLD_HANDLER} = 1;
+
   my $eval_stat;
   eval {
     syslog($level, "%s", $msg); 1;
   } or do {
     $eval_stat = $@ ne '' ? $@ : "errno=$!";  chomp $eval_stat;
   };
-  $Mail::SpamAssassin::Logger::LOG_SA{INHIBIT_LOGGING_IN_SIGCHLD_HANDLER} = 0;
+
+# no longer needed since a patch to bug 6745:
+# $Mail::SpamAssassin::Logger::LOG_SA{INHIBIT_LOGGING_IN_SIGCHLD_HANDLER} = 0;
 
   if (defined $eval_stat) {
     if ($self->check_syslog_sigpipe($msg)) {

Modified: spamassassin/trunk/spamd/spamd.raw
URL: http://svn.apache.org/viewvc/spamassassin/trunk/spamd/spamd.raw?rev=1338278&r1=1338277&r2=1338278&view=diff
==============================================================================
--- spamassassin/trunk/spamd/spamd.raw (original)
+++ spamassassin/trunk/spamd/spamd.raw Mon May 14 16:31:09 2012
@@ -589,6 +589,7 @@ my $timeout_tcp;          # socket timeo
 my $timeout_child;        # processing timeout (headers->finish), 0=no timeout
 my $clients_per_child;    # number of clients each child should process
 my %children;             # current children
+my @children_exited;
 
 if ( defined $opt{'max-children'} ) {
   $childlimit = $opt{'max-children'};
@@ -1033,6 +1034,8 @@ while (1) {
 # child_handler()  if !$scaling || am_running_on_windows();
   child_handler();  # it doesn't hurt to call child_handler unconditionally
 
+  child_cleaner();
+
   do_sighup_restart()  if defined $got_sighup;
 
   for (my $i = keys %children; $i < $childlimit; $i++) {
@@ -2523,7 +2526,8 @@ sub child_handler {
   my ($sig) = @_;
 
   # do NOT call syslog here unless the child's pid is in our list of known
-  # children.  This is due to syslog-ng brokenness -- bugs 3625, 4237.
+  # children.  This is due to syslog-ng brokenness -- bugs 3625, 4237;
+  # see also bug 6745.
 
   # clean up any children which have exited
   for (;;) {
@@ -2534,12 +2538,23 @@ sub child_handler {
     #
     my $pid = waitpid(-1, WNOHANG);
     last if !$pid || $pid == -1;
-    my $child_stat = $?;
+    push(@children_exited, [$pid, $?, $sig, time]);
+  }
 
-    if (!defined $children{$pid}) {
-      # ignore this child; we didn't realise we'd forked it. bug 4237
-      next;
-    }
+  $SIG{CHLD} = \&child_handler;    # reset as necessary, should be at end
+}
+
+# takes care of dead children, as noted by a child_handler()
+# called in a main program flow (not from a signal handler)
+#
+sub child_cleaner {
+  while (@children_exited) {
+    my $tuple = shift(@children_exited);
+    next if !$tuple;  # just in case
+    my($pid, $child_stat, $sig, $timestamp) = @$tuple;
+
+    # ignore this child if we didn't realise we'd forked it. bug 4237
+    next if !defined $children{$pid};
 
     # remove them from our child listing
     delete $children{$pid};
@@ -2550,15 +2565,10 @@ sub child_handler {
       my $sock = $backchannel->get_socket_for_child($pid);
       if ($sock) { $sock->close(); }
     }
-
-    unless ($Mail::SpamAssassin::Logger::LOG_SA{INHIBIT_LOGGING_IN_SIGCHLD_HANDLER}) {
-      info("spamd: handled cleanup of child pid [%s]%s: %s",
-           $pid, (defined $sig ? " due to SIG$sig" : ""),
-           exit_status_str($child_stat,0));
-    }
+    info("spamd: handled cleanup of child pid [%s]%s: %s",
+         $pid, (defined $sig ? " due to SIG$sig" : ""),
+         exit_status_str($child_stat,0));
   }
-
-  $SIG{CHLD} = \&child_handler;    # reset as necessary, should be at end
 }
 
 sub restart_handler {