| Subject: | Sendmail::PMilter violates Milter protocol when max children reached |
When the ithread or postfork dispatcher finds that it has reached the maximum number of children, it sends SMFIR_TEMPFAIL to the MTA rather than negotating as it's supposed to.
This isn't allowed; the milter is required to negotiate at that point, and SMFIR_TEMPFAIL Is not a valid message to send. Sendmail complains in the logs every time it happens.
If PMilter _must_ return a temporary failure like that, then it will need to be done in main subroutine of Sendmail::PMilter::Context (which, I know, sort of defeats the purpose, since you have to start a child before that code even runs). However, I don't think that's the correct behavior. The correct behavior, in my opinion, is that PMilter should block until a slot becomes available.
The attached patch implements that. Please note: I have tested the changes to postfork_dispatcher but I have _not_ tested the changes to ithread_dispatcher.
| Subject: | maxchildren.patch |
--- PMilter.pm 2013/06/03 14:09:25 1.2
+++ PMilter.pm 2013/06/03 16:17:42 1.3
@@ -1,4 +1,4 @@
-# $Id: PMilter.pm,v 1.2 2013/06/03 14:09:25 root Exp $
+# $Id: PMilter.pm,v 1.3 2013/06/03 16:17:42 root Exp $
#
# Copyright (c) 2002-2004 Todd Vierling <tv@pobox.com> <tv@duh.org>
# All rights reserved.
@@ -44,6 +44,7 @@
use Sendmail::Milter 0.18; # get needed constants
use Socket;
use Symbol;
+use Time::HiRes 'time';
use UNIVERSAL;
our $VERSION = '1.00';
@@ -654,6 +655,7 @@
sub ithread_dispatcher {
require threads;
require threads::shared;
+ require Thread::Semaphore;
my $nchildren = 0;
@@ -664,6 +666,11 @@
my $lsocket = shift;
my $handler = shift;
my $maxchildren = $this->get_max_interpreters();
+ my $child_sem;
+
+ if ($maxchildren) {
+ $child_sem = Thread::Semaphore->new($maxchildren);
+ }
my $siginfo = exists($SIG{INFO}) ? 'INFO' : 'USR1';
local $SIG{$siginfo} = sub {
@@ -681,6 +688,9 @@
lock($nchildren);
$nchildren--;
+ if ($child_sem) {
+ $child_sem->up();
+ }
warn $died if $died;
};
@@ -690,18 +700,12 @@
warn "$$: incoming connection\n" if ($DEBUG > 0);
- # If the load's too high, fail and go back to top of loop.
- if ($maxchildren) {
- my $cnchildren = $nchildren; # make constant
-
- if ($cnchildren >= $maxchildren) {
- warn "load too high: children $cnchildren >= max $maxchildren";
-
- $socket->autoflush(1);
- $socket->print(pack('N/a*', 't')); # SMFIR_TEMPFAIL
- $socket->close();
- next;
- }
+ if ($child_sem and ! $child_sem->down_nb()) {
+ warn "pausing for high load: children $nchildren >= max $maxchildren";
+ my $start = time();
+ $child_sem->down();
+ my $end = time();
+ warn sprintf("paused for %.1f seconds due to high load", $end - $start);
}
# scoping block for lock()
@@ -867,6 +871,10 @@
otherwise mostly idle mail traffic, as the idle-time resource consumption is
very low.
+If the maximum number of interpreters is running when a new connection
+comes in, this dispatcher blocks until a slot becomes available for a
+new interpreter.
+
=cut
sub postfork_dispatcher () {
@@ -900,17 +908,22 @@
warn "$$: incoming connection\n" if ($DEBUG > 0);
# If the load's too high, fail and go back to top of loop.
- if ($maxchildren) {
+ my $paused = undef;
+ while ($maxchildren) {
my $cnchildren = $nchildren; # make constant
if ($cnchildren >= $maxchildren) {
- warn "load too high: children $cnchildren >= max $maxchildren";
-
- $socket->autoflush(1);
- $socket->print(pack('N/a*', 't')); # SMFIR_TEMPFAIL
- $socket->close();
- next;
+ warn "pausing for high load: children $cnchildren >= max $maxchildren";
+ $paused = time() if (! $paused);
+ pause();
}
+ else {
+ last;
+ }
+ }
+
+ if ($paused) {
+ warn sprintf("paused for %.1f seconds due to high load", time() - $paused);
}
my $pid = fork();