From: Philip Hazel Date: Wed, 16 Feb 2005 15:24:21 +0000 (+0000) Subject: Fix problem with smtp_max_per_host; test for completed processes before X-Git-Tag: exim-4_50~10 X-Git-Url: https://git.exim.org/users/heiko/exim.git/commitdiff_plain/bb6e88ff38009f30d0483414fcabc97f00042f17?ds=inline 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 77e858762..b88fbea69 100644 --- 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 ------------------------------------------- @@ -367,6 +367,14 @@ Exim version 4.50 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. diff --git a/src/src/daemon.c b/src/src/daemon.c index caf8cb24e..2495e18ae 100644 --- 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 * @@ -39,8 +39,8 @@ static int accept_retry_errno; 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; @@ -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); - int status; 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) { - int sk, lcount; + int sk, lcount, select_errno; int max_socket = 0; BOOL select_failed = FALSE; fd_set select_listen; @@ -1660,6 +1725,17 @@ for (;;) 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. */ @@ -1754,57 +1830,7 @@ for (;;) 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