Fix problem with smtp_max_per_host; test for completed processes before

looking at the count for a new connection.

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 77e858762d552ada0ade90b9c86d6c211a35c686..b88fbea69cb770eb9c8fc225cc97b23236cf28d0 100644 (file)
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -1,4 +1,4 @@
-$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.78 2005/02/15 09:31:13 ph10 Exp $
+$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.79 2005/02/16 15:24:21 ph10 Exp $

 
 Change log file for Exim from version 4.21
 -------------------------------------------
 
 Change log file for Exim from version 4.21
 -------------------------------------------


@@ -367,6 +367,14 @@ Exim version 4.50
 
 78. Error message wording change in sieve.c.
 
 
 78. Error message wording change in sieve.c.
 

+79. If smtp_accept_max_per_host was set, the number of connections could be
+    restricted to fewer than expected, because the daemon was trying to set up
+    a new connection before checking whether the processes handling previous
+    connections had finished. The check for completed processes is now done
+    earlier. On busy systems, this bug wouldn't be noticed because something
+    else would have woken the daemon, and it would have reaped the completed
+    process earlier.
+

 
 ----------------------------------------------------
 See the note above about the 4.44 and 4.50 releases.
 
 ----------------------------------------------------
 See the note above about the 4.44 and 4.50 releases.

diff --git a/src/src/daemon.c b/src/src/daemon.c
index caf8cb24e08cb70da9a9b7195beed3bc83b8e71b..2495e18ae4a2059e83727f6af3f0f184bb37cd9c 100644 (file)
--- a/src/src/daemon.c
+++ b/src/src/daemon.c
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/src/src/daemon.c,v 1.7 2005/01/27 15:15:30 ph10 Exp $ */
+/* $Cambridge: exim/src/src/daemon.c,v 1.8 2005/02/16 15:24:21 ph10 Exp $ */

 
 /*************************************************
 *     Exim - an Internet mail transport agent    *
 
 /*************************************************
 *     Exim - an Internet mail transport agent    *


@@ -39,8 +39,8 @@ static int   accept_retry_errno;
 static BOOL  accept_retry_select_failed;
 
 static int   queue_run_count = 0;
 static BOOL  accept_retry_select_failed;
 
 static int   queue_run_count = 0;

-static pid_t *queue_pid_slots;
-static smtp_slot *smtp_slots;
+static pid_t *queue_pid_slots = NULL;
+static smtp_slot *smtp_slots = NULL;

 
 static BOOL  write_pid = TRUE;
 
 
 static BOOL  write_pid = TRUE;
 


@@ -776,6 +776,72 @@ return FALSE;
 
 
 
 
 
 

+/*************************************************
+*         Handle terminating subprocesses        *
+*************************************************/
+
+/* Handle the termination of child processes. Theoretically, this need be done
+only when sigchld_seen is TRUE, but rumour has it that some systems lose
+SIGCHLD signals at busy times, so to be on the safe side, this function is
+called each time round. It shouldn't be too expensive.
+
+Arguments:  none
+Returns:    nothing
+*/
+
+static void
+handle_ending_processes(void)
+{
+int status;
+pid_t pid;
+
+while ((pid = waitpid(-1, &status, WNOHANG)) > 0)
+  {
+  int i;
+  DEBUG(D_any) debug_printf("child %d ended: status=0x%x\n", (int)pid,
+    status);
+
+  /* If it's a listening daemon for which we are keeping track of individual 
+  subprocesses, deal with an accepting process that has terminated. */
+
+  if (smtp_slots != NULL)
+    {
+    for (i = 0; i < smtp_accept_max; i++)
+      {
+      if (smtp_slots[i].pid == pid)
+        {
+        if (smtp_slots[i].host_address != NULL)
+          store_free(smtp_slots[i].host_address);
+        smtp_slots[i] = empty_smtp_slot;
+        if (--smtp_accept_count < 0) smtp_accept_count = 0;
+        DEBUG(D_any) debug_printf("%d SMTP accept process%s now running\n",
+          smtp_accept_count, (smtp_accept_count == 1)? "" : "es");
+        break;
+        }
+      }
+    if (i < smtp_accept_max) continue;  /* Found an accepting process */
+    }
+
+  /* If it wasn't an accepting process, see if it was a queue-runner
+  process that we are tracking. */
+
+  if (queue_pid_slots != NULL)
+    {
+    for (i = 0; i < queue_run_max; i++)
+      {
+      if (queue_pid_slots[i] == pid)
+        {
+        queue_pid_slots[i] = 0;
+        if (--queue_run_count < 0) queue_run_count = 0;
+        DEBUG(D_any) debug_printf("%d queue-runner process%s now running\n",
+          queue_run_count, (queue_run_count == 1)? "" : "es");
+        break;
+        }
+      }
+    }
+  }
+}
+

 
 
 /*************************************************
 
 
 /*************************************************


@@ -1513,7 +1579,6 @@ for (;;)
   #endif
 
   SOCKLEN_T len = sizeof(accepted);
   #endif
 
   SOCKLEN_T len = sizeof(accepted);

-  int status;

   pid_t pid;
 
   /* This code is placed first in the loop, so that it gets obeyed at the
   pid_t pid;
 
   /* This code is placed first in the loop, so that it gets obeyed at the


@@ -1622,7 +1687,7 @@ for (;;)
 
   if (daemon_listen)
     {
 
   if (daemon_listen)
     {

-    int sk, lcount;
+    int sk, lcount, select_errno;

     int max_socket = 0;
     BOOL select_failed = FALSE;
     fd_set select_listen;
     int max_socket = 0;
     BOOL select_failed = FALSE;
     fd_set select_listen;


@@ -1660,6 +1725,17 @@ for (;;)
       lcount = 1;
       }
       
       lcount = 1;
       }
       

+    /* Clean up any subprocesses that may have terminated. We need to do this 
+    here so that smtp_accept_max_per_host works when a connection to that host 
+    has completed, and we are about to accept a new one. When this code was 
+    later in the sequence, a new connection could be rejected, even though an 
+    old one had just finished. Preserve the errno from any select() failure for 
+    the use of the common select/accept error processing below. */
+    
+    select_errno = errno;
+    handle_ending_processes();
+    errno = select_errno; 
+      

     /* Loop for all the sockets that are currently ready to go. If select
     actually failed, we have set the count to 1 and select_failed=TRUE, so as
     to use the common error code for select/accept below. */
     /* Loop for all the sockets that are currently ready to go. If select
     actually failed, we have set the count to 1 and select_failed=TRUE, so as
     to use the common error code for select/accept below. */


@@ -1754,57 +1830,7 @@ for (;;)
     tv.tv_sec = queue_interval;
     tv.tv_usec = 0;
     select(0, NULL, NULL, NULL, &tv);
     tv.tv_sec = queue_interval;
     tv.tv_usec = 0;
     select(0, NULL, NULL, NULL, &tv);

-    }
-
-  /* Handle the termination of a child process. Theoretically, this need
-  be done only when sigchld_seen is TRUE, but rumour has it that some systems
-  lose SIGCHLD signals at busy times, so to be on the safe side, just
-  do it each time round. It shouldn't be too expensive. */
-
-  while ((pid = waitpid(-1, &status, WNOHANG)) > 0)
-    {
-    int i;
-    DEBUG(D_any) debug_printf("child %d ended: status=0x%x\n", (int)pid,
-      status);
-
-    /* If it's a listening daemon, deal with an accepting process that has
-    terminated. */
-
-    if (daemon_listen)
-      {
-      for (i = 0; i < smtp_accept_max; i++)
-        {
-        if (smtp_slots[i].pid == pid)
-          {
-          if (smtp_slots[i].host_address != NULL)
-            store_free(smtp_slots[i].host_address);
-          smtp_slots[i] = empty_smtp_slot;
-          if (--smtp_accept_count < 0) smtp_accept_count = 0;
-          DEBUG(D_any) debug_printf("%d SMTP accept process%s now running\n",
-            smtp_accept_count, (smtp_accept_count == 1)? "" : "es");
-          break;
-          }
-        }
-      if (i < smtp_accept_max) continue;  /* Found an accepting process */
-      }
-
-    /* If it wasn't an accepting process, see if it was a queue-runner
-    process, if we are keeping track of them. */
-
-    if (queue_interval > 0)
-      {
-      for (i = 0; i < queue_run_max; i++)
-        {
-        if (queue_pid_slots[i] == pid)
-          {
-          queue_pid_slots[i] = 0;
-          if (--queue_run_count < 0) queue_run_count = 0;
-          DEBUG(D_any) debug_printf("%d queue-runner process%s now running\n",
-            queue_run_count, (queue_run_count == 1)? "" : "es");
-          break;
-          }
-        }
-      }
+    handle_ending_processes(); 

     }
 
   /* Re-enable the SIGCHLD handler if it has been run. It can't do it
     }
 
   /* Re-enable the SIGCHLD handler if it has been run. It can't do it