You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spamassassin.apache.org by jm...@apache.org on 2006/10/04 13:35:50 UTC

svn commit: r452849 - in /spamassassin/trunk: MANIFEST spamd/spamd.raw t/spamd_kill_restart.t t/spamd_kill_restart_rr.t

Author: jm
Date: Wed Oct  4 04:35:49 2006
New Revision: 452849

URL: http://svn.apache.org/viewvc?view=rev&rev=452849
Log:
bug 4304: in some setups, eg where a plugin implements 'spamd_child_init' with a long-running task, it's possible for child spamd processes to take too long to exit when the parent spamd is killed with SIGINT or SIGTERM.  if the caller then starts a new spamd immediately, it will die with 'address already in use', since the old children are still running.  Fix this race by unblocking the signals as soon as possible in child spamds after they are forked, and before calling the plugin hook; also add test cases

Added:
    spamassassin/trunk/t/spamd_kill_restart.t   (with props)
    spamassassin/trunk/t/spamd_kill_restart_rr.t   (with props)
Modified:
    spamassassin/trunk/MANIFEST
    spamassassin/trunk/spamd/spamd.raw

Modified: spamassassin/trunk/MANIFEST
URL: http://svn.apache.org/viewvc/spamassassin/trunk/MANIFEST?view=diff&rev=452849&r1=452848&r2=452849
==============================================================================
--- spamassassin/trunk/MANIFEST (original)
+++ spamassassin/trunk/MANIFEST Wed Oct  4 04:35:49 2006
@@ -521,3 +521,5 @@
 t/uribl.t
 t/shortcircuit.t
 t/spamc_y.t
+t/spamd_kill_restart.t
+t/spamd_kill_restart_rr.t

Modified: spamassassin/trunk/spamd/spamd.raw
URL: http://svn.apache.org/viewvc/spamassassin/trunk/spamd/spamd.raw?view=diff&rev=452849&r1=452848&r2=452849
==============================================================================
--- spamassassin/trunk/spamd/spamd.raw (original)
+++ spamassassin/trunk/spamd/spamd.raw Wed Oct  4 04:35:49 2006
@@ -891,6 +891,12 @@
   else {
     ## CHILD
 
+    # Reset signal handling to default settings, and unblock.
+    # These lines must be as soon as possible after the fork (bug 4304)
+    setup_child_sig_handlers();
+    sigprocmask( POSIX::SIG_UNBLOCK(), $sigset )
+      or die "spamd: cannot unblock SIGINT/SIGCHLD for fork: $!\n";
+
     $spamtest->call_plugins("spamd_child_init");
 
     # support non-root use
@@ -926,13 +932,6 @@
         die "spamd: setuid to uid $uuid failed\n";
       }
     }
-
-    # Reset signal handling to default settings.
-    setup_child_sig_handlers();
-
-    # unblock signals
-    sigprocmask( POSIX::SIG_UNBLOCK(), $sigset )
-      or die "spamd: cannot unblock SIGINT/SIGCHLD for fork: $!\n";
 
     # set process name where supported
     # this will help make it clear via process listing which is child/parent

Added: spamassassin/trunk/t/spamd_kill_restart.t
URL: http://svn.apache.org/viewvc/spamassassin/trunk/t/spamd_kill_restart.t?view=auto&rev=452849
==============================================================================
--- spamassassin/trunk/t/spamd_kill_restart.t (added)
+++ spamassassin/trunk/t/spamd_kill_restart.t Wed Oct  4 04:35:49 2006
@@ -0,0 +1,84 @@
+#!/usr/bin/perl
+
+use lib '.'; use lib 't';
+use SATest; sa_t_init("spamd_kill_restart");
+use constant TEST_ENABLED => !$SKIP_SPAMD_TESTS && !$RUNNING_ON_WINDOWS;
+
+use Test; BEGIN { plan tests => (TEST_ENABLED? 63 : 0) };
+
+use File::Spec;
+
+exit unless TEST_ENABLED;
+
+# ---------------------------------------------------------------------------
+
+my $pid_file = "log/spamd.pid";
+
+my($pid1, $pid2);
+
+print "Starting spamd...\n";
+start_spamd("-L -r ${pid_file}");
+sleep 1;
+
+for $retry (0 .. 9) {
+  ok ($pid1 = get_pid());
+  print "killing spamd at pid $pid1, loop try $retry...\n";
+  ok ($pid1 != 0 and kill ('INT', $pid1));
+
+# override these so the old logs are still visible and the new
+# spamd will be started even though stop_spamd() was not called
+  $spamd_pid = 0;
+  $testname = "spamd_kill_restart_retry_".$retry;
+
+  print "starting new spamd, loop try $retry...\n";
+  start_spamd("-D -L -r ${pid_file}");
+  ok ($pid1 = get_pid());
+
+  print "Waiting for spamd at pid $pid1 to restart...\n";
+# note that the wait period increases the longer it takes,
+# 20 retries works out to a total of 60 seconds
+  my $timeout = 20;
+  my $wait = 0;
+  do {
+    sleep (int($wait++ / 4) + 1) if $timeout > 0;
+    $timeout--;
+  } while(!-e $pid_file && $timeout);
+  ok (-e $pid_file);
+
+  ok ($pid2 = get_pid($pid1));
+  print "Looking for new spamd at pid $pid2...\n";
+  ok ($pid2 != 0 and kill (0, $pid2));
+
+  $pid1 = $pid2;
+}
+
+  print "Checking GTUBE...\n";
+  %patterns = (
+    q{ X-Spam-Flag: YES } => 'flag',
+    q{ GTUBE }            => 'gtube',
+  );
+  ok (spamcrun ("< data/spam/gtube.eml", \&patterns_run_cb));
+  ok_all_patterns;
+
+
+print "Stopping spamd...\n";
+stop_spamd;
+
+
+sub get_pid {
+  my($opid, $npid) = (@_, 0, 0);
+  #my $timeout = 5;
+  #do {
+  #  sleep 1;
+  #  $timeout--;
+
+    if (open (PID, "< ${pid_file}")) {
+      $npid = <PID>;
+      chomp $npid;
+      close(PID);
+    } else {
+      die "Could not open pid file ${pid_file}: $!\n";
+    }
+  #} until ($npid != $opid or $timeout == 0);
+  return $npid;
+}

Propchange: spamassassin/trunk/t/spamd_kill_restart.t
------------------------------------------------------------------------------
    svn:executable = *

Added: spamassassin/trunk/t/spamd_kill_restart_rr.t
URL: http://svn.apache.org/viewvc/spamassassin/trunk/t/spamd_kill_restart_rr.t?view=auto&rev=452849
==============================================================================
--- spamassassin/trunk/t/spamd_kill_restart_rr.t (added)
+++ spamassassin/trunk/t/spamd_kill_restart_rr.t Wed Oct  4 04:35:49 2006
@@ -0,0 +1,84 @@
+#!/usr/bin/perl
+
+use lib '.'; use lib 't';
+use SATest; sa_t_init("spamd_kill_restart_rr");
+use constant TEST_ENABLED => !$SKIP_SPAMD_TESTS && !$RUNNING_ON_WINDOWS;
+
+use Test; BEGIN { plan tests => (TEST_ENABLED? 63 : 0) };
+
+use File::Spec;
+
+exit unless TEST_ENABLED;
+
+# ---------------------------------------------------------------------------
+
+my $pid_file = "log/spamd.pid";
+
+my($pid1, $pid2);
+
+print "Starting spamd...\n";
+start_spamd("-L --round-robin -r ${pid_file}");
+sleep 1;
+
+for $retry (0 .. 9) {
+  ok ($pid1 = get_pid());
+  print "killing spamd at pid $pid1, loop try $retry...\n";
+  ok ($pid1 != 0 and kill ('INT', $pid1));
+
+# override these so the old logs are still visible and the new
+# spamd will be started even though stop_spamd() was not called
+  $spamd_pid = 0;
+  $testname = "spamd_kill_restart_rr_retry_".$retry;
+
+  print "starting new spamd, loop try $retry...\n";
+  start_spamd("-D -L --round-robin -r ${pid_file}");
+  ok ($pid1 = get_pid());
+
+  print "Waiting for spamd at pid $pid1 to restart...\n";
+# note that the wait period increases the longer it takes,
+# 20 retries works out to a total of 60 seconds
+  my $timeout = 20;
+  my $wait = 0;
+  do {
+    sleep (int($wait++ / 4) + 1) if $timeout > 0;
+    $timeout--;
+  } while(!-e $pid_file && $timeout);
+  ok (-e $pid_file);
+
+  ok ($pid2 = get_pid($pid1));
+  print "Looking for new spamd at pid $pid2...\n";
+  ok ($pid2 != 0 and kill (0, $pid2));
+
+  $pid1 = $pid2;
+}
+
+  print "Checking GTUBE...\n";
+  %patterns = (
+    q{ X-Spam-Flag: YES } => 'flag',
+    q{ GTUBE }            => 'gtube',
+  );
+  ok (spamcrun ("< data/spam/gtube.eml", \&patterns_run_cb));
+  ok_all_patterns;
+
+
+print "Stopping spamd...\n";
+stop_spamd;
+
+
+sub get_pid {
+  my($opid, $npid) = (@_, 0, 0);
+  #my $timeout = 5;
+  #do {
+  #  sleep 1;
+  #  $timeout--;
+
+    if (open (PID, "< ${pid_file}")) {
+      $npid = <PID>;
+      chomp $npid;
+      close(PID);
+    } else {
+      die "Could not open pid file ${pid_file}: $!\n";
+    }
+  #} until ($npid != $opid or $timeout == 0);
+  return $npid;
+}

Propchange: spamassassin/trunk/t/spamd_kill_restart_rr.t
------------------------------------------------------------------------------
    svn:executable = *