Many changes to make message delivery more robust in the face of

failures, bugs, and lock acquisition timeouts.  Instead of storing
information about the progress of the delivery on the Message object,
we pass around a parallel data structure called `msgdata' (current
just a dictionary).  All calculated information is passed through this
object, but this changes the API to handler modules.  They now take
three arguments: the mailing list, the message object, and the
msgdata.  WARNING: This may change before 2.0 final.

Specific changes include:

HandlerAPI

    DiscardMessage(), HandlerAPI.SomeRecipientsFailed(): New shared
    exceptions.

    pipeline_deliver(): removed

    LIST_PIPELINE: global containing the primary list delivery
    pipeline

    DelivertoList: Revamped main entry point into message delivery to
    list membership.  Takes three arguments: the mailing list, the
    message object, and the msgdata dictionary. This digs the
    pipeline to use out of the msgdata (allowing resumption of
    prematurely interrupted pipeline deliveries).

    Then each module is called in turn, and the shared exceptions are
    caught.  As each module is completed successfully, it is removed
    from the head of the pipeline.  This function returns the number
    of pipeline modules remaining to be executed (i.e. a return of 0
    means DeliverToList() is done with this message and it can be
    dequeued).

    A catch-all is included in case some unexpected exception occurs
    (say a bug or typo in one of the delivery modules).  Such an error
    will queue the message, so at least it doesn't just get lost.  We
    try to never just lose a message.

    RedeliverMessage(), DeliverToUser(): reimplemented in terms of
    DeliverToList().

Acknowledge, AfterDelivery, CalcRecips, Cleanse, CookHeaders,
Decorate, Replybot, ToArchive, ToUsenet

    Fix the function signature to match the new API (three arguments),
    and changed the implementations to extract delivery information
    from msgdata instead of as attributes of the message object.

Approved

    Same as above, but also removed NotApproved exception.   LoopError
    is now multiply derived from HandlerAPI.DiscardMessage and
    Errors.MMLoopingPost.

Hold

    Same as above, but also changed slightly the way an exception is
    raised when a message is held.  hold_for_approval() now takes four
    arguments (the msgdata parameter has been added), and the exc
    object can be a class or instance.  If it's a class, it is simply
    zero-arg'd instantiated.  We also use the str() of the exception
    to get us the reason for the hold.  This allows us to override
    HandlerAPI.MessageHeld.__str__() for MessageToBig so that we can
    include the size of the message being held.

SMTPDirect

    Same as above, but instead of explicitly enqueuing the messages
    when some or all of the recipient deliveries failed, just raise a
    HandlerAPI.SomeRecipientsFailed exception and let DeliverToList()
    manage the enqueuing.  Thus queue_message() is removed.

Sendmail

    Same as above, but if any chunks fail delivery, those recipients
    are queued by raising SomeRecipientsFailed.

SpamDetect

    Same as above, except that if a regexp matches, a SpamDetect
    exception is raised directly.  The DeliverToList() framework
    discards these spam messages instead of holding them for
    approval.

ToDigest

    Same as above, except that if a digest is prepared for delivery,
    it is not sent directly via mlist.Post().  Instead, the message is
    queued for delivery, thereby relinquishing the lock soon.  This
    means that digests will only be sent the next time qrunner runs.

16 files changed, 232 insertions, 204 deletions

diff --git a/Mailman/Handlers/Acknowledge.py b/Mailman/Handlers/Acknowledge.py
index f94cb79c2..fcc8174ec 100644
--- a/Mailman/Handlers/Acknowledge.py
+++ b/Mailman/Handlers/Acknowledge.py
@@ -29,8 +29,8 @@ from Mailman.Handlers import HandlerAPI
 
 
 
-def process(mlist, msg):
-    sender = getattr(msg, 'original_sender', msg.GetSender())
+def process(mlist, msg, msgdata):
+    sender = msgdata.get('original_sender', msg.GetSender())
     sender = mlist.FindUser(sender)
     if sender and mlist.GetUserOption(sender, mm_cfg.AcknowledgePosts):
         subject = msg.getheader('subject')
diff --git a/Mailman/Handlers/AfterDelivery.py b/Mailman/Handlers/AfterDelivery.py
index 7bf0c3f46..f7707a378 100644
--- a/Mailman/Handlers/AfterDelivery.py
+++ b/Mailman/Handlers/AfterDelivery.py
@@ -23,6 +23,6 @@ import time
 
 
 
-def process(mlist, msg):
+def process(mlist, msg, msgdata):
     mlist.last_post_time = time.time()
     mlist.post_id = mlist.post_id + 1
diff --git a/Mailman/Handlers/Approve.py b/Mailman/Handlers/Approve.py
index b61603a54..ee96bce0b 100644
--- a/Mailman/Handlers/Approve.py
+++ b/Mailman/Handlers/Approve.py
@@ -28,20 +28,15 @@ import HandlerAPI
 from Mailman import mm_cfg
 from Mailman import Errors
 
-
-class NotApproved(HandlerAPI.HandlerError):
-    pass
-
-
 # multiple inheritance for backwards compatibility
-class LoopError(NotApproved, Errors.MMLoopingPost):
-    pass
+class LoopError(HandlerAPI.DiscardMessage, Errors.MMLoopingPost):
+    """We've seen this message before"""
 
 
 
-def process(mlist, msg):
+def process(mlist, msg, msgdata):
     # short circuits
-    if getattr(msg, 'approved', 0):
+    if msgdata.get('approved'):
         # digests, Usenet postings, and some other messages come
         # pre-approved.  TBD: we may want to further filter Usenet messages,
         # so the test above may not be entirely correct.
@@ -52,7 +47,7 @@ def process(mlist, msg):
         # TBD: should we definitely deny if the password exists but does not
         # match?  For now we'll let it percolate up for further
         # determination.
-        msg.approved = 1
+        msgdata['approved'] = 1
     # has this message already been posted to this list?
     beentheres = map(filterfunc, msg.getallmatchingheaders('x-beenthere'))
     if mlist.GetListEmail() in beentheres:
diff --git a/Mailman/Handlers/CalcRecips.py b/Mailman/Handlers/CalcRecips.py
index 26f0d059c..34520f186 100644
--- a/Mailman/Handlers/CalcRecips.py
+++ b/Mailman/Handlers/CalcRecips.py
@@ -27,10 +27,10 @@ from Mailman import mm_cfg
 
 
 
-def process(mlist, msg):
-    # yes, short circuit if the message object already has a recipients
-    # attribute, regardless of whether the list is empty or not.
-    if hasattr(msg, 'recips'):
+def process(mlist, msg, msgdata):
+    # Short circuit if we've already calculated the recipients list,
+    # regardless of whether the list is empty or not.
+    if msgdata.has_key('recips'):
         return
     dont_send_to_sender = 0
     # Get the membership address of the sender, if a member.  Then get the
@@ -53,4 +53,4 @@ def process(mlist, msg):
             # (not metoo), but delivery to their address is disabled (nomail)
             pass
     # bookkeeping
-    msg.recips = recips
+    msgdata['recips'] = recips
diff --git a/Mailman/Handlers/Cleanse.py b/Mailman/Handlers/Cleanse.py
index a6d14375b..408602a77 100644
--- a/Mailman/Handlers/Cleanse.py
+++ b/Mailman/Handlers/Cleanse.py
@@ -17,12 +17,13 @@
 """Cleanse certain headers from all messages."""
 
 
-def process(mlist, msg):
-    # Always remove this header from any outgoing messages, but be sure to do
-    # this before the information on the header is actually used.
+def process(mlist, msg, msgdata):
+    # Always remove this header from any outgoing messages.  Be sure to do
+    # this after the information on the header is actually used, but before a
+    # permanent record of the header is saved.
     del msg['approved']
     #
-    # We remove other headers for anonymous lists
+    # We remove other headers from anonymous lists
     if mlist.anonymous_list:
         del msg['reply-to']
         del msg['sender']
diff --git a/Mailman/Handlers/CookHeaders.py b/Mailman/Handlers/CookHeaders.py
index b49175570..05c19edc4 100644
--- a/Mailman/Handlers/CookHeaders.py
+++ b/Mailman/Handlers/CookHeaders.py
@@ -24,15 +24,16 @@ from Mailman import mm_cfg
 
 
 
-def process(mlist, msg):
+def process(mlist, msg, msgdata):
     # Because we're going to modify various important headers in the email
-    # message, we want to save some of the information as attributes for
-    # later.  Specifically, the sender header will get waxed, but we need it
-    # for the Acknowledge module later.
-    msg.original_sender = msg.GetSender()
+    # message, we want to save some of the information in the msgdata
+    # dictionary for later.  Specifically, the sender header will get waxed,
+    # but we need it for the Acknowledge module later.
+    msgdata['original_sender'] = msg.GetSender()
     subject = msg.getheader('subject')
     adminaddr = mlist.GetAdminEmail()
-    if not getattr(msg, 'isdigest', 0) and not getattr(msg, 'fasttrack', 0):
+    fasttrack = msgdata.get('fasttrack')
+    if not msgdata.get('isdigest') and not fasttrack:
         # Add the subject prefix unless the message is a digest or is being
         # fast tracked (e.g. internally crafted, delivered to a single user
         # such as the list admin).  We assume all digests have an appropriate
@@ -70,7 +71,7 @@ def process(mlist, msg):
     #
     # Reply-To: munging.  Do not do this if the message is "fast tracked",
     # meaning it is internally crafted and delivered to a specific user.
-    if not getattr(msg, 'fasttrack', 0):
+    if not fasttrack:
         # Set Reply-To: header to point back to this list
         if mlist.reply_goes_to_list == 1:
             msg['Reply-To'] = mlist.GetListEmail()
diff --git a/Mailman/Handlers/Decorate.py b/Mailman/Handlers/Decorate.py
index a7e1d11e7..122da2ff0 100644
--- a/Mailman/Handlers/Decorate.py
+++ b/Mailman/Handlers/Decorate.py
@@ -23,7 +23,7 @@ import string
 
 
 
-def process(mlist, msg):
+def process(mlist, msg, msgdata):
     d = Utils.SafeDict(mlist.__dict__)
     d['cgiext'] = mm_cfg.CGIEXT
     # interpolate into the header
diff --git a/Mailman/Handlers/HandlerAPI.py b/Mailman/Handlers/HandlerAPI.py
index f098929fd..aba0e1a09 100644
--- a/Mailman/Handlers/HandlerAPI.py
+++ b/Mailman/Handlers/HandlerAPI.py
@@ -16,57 +16,122 @@
 
 """Contains all the common functionality for the msg handler API."""
 
+import traceback
+import time
+
 from Mailman import mm_cfg
 from Mailman import Errors
+from Mailman.pythonlib.StringIO import StringIO
+
 
+
+# Exception classes for this subsystem.
 class HandlerError(Errors.MailmanError):
     """Base class for all handler errors."""
-    pass
 
 class MessageHeld(HandlerError):
     """Base class for all message-being-held short circuits."""
-    pass
+    def __str__(self):
+        return self.__class__.__doc__
+
+class DiscardMessage(HandlerError):
+    """The message can be discarded with no further action"""
+
+class SomeRecipientsFailed(HandlerError):
+    """Delivery to some or all recipients failed"""
+
+
+
+# All messages which are delivered to the entire list membership go through
+# this pipeline of handler modules.
+LIST_PIPELINE = ['SpamDetect',
+                 'Approve',
+                 'Replybot',
+                 'Hold',
+                 'Cleanse',
+                 'CookHeaders',
+                 'ToDigest',
+                 'ToArchive',
+                 'ToUsenet',
+                 'CalcRecips',
+                 'Decorate',
+                 mm_cfg.DELIVERY_MODULE,
+                 'AfterDelivery',
+                 'Acknowledge',
+                 ]
 
 
 
-def pipeline_delivery(mlist, msg, pipeline):
-    for modname in pipeline:
-        mod = __import__('Mailman.Handlers.'+modname)
+# Central mail delivery handler
+def DeliverToList(mlist, msg, msgdata):
+    pipeline = msgdata.get('pipeline', LIST_PIPELINE)
+    while pipeline:
+        modname = pipeline.pop(0)
+        mod = __import__('Mailman.Handlers.' + modname)
         func = getattr(getattr(getattr(mod, 'Handlers'), modname), 'process')
         try:
-            func(mlist, msg)
+            mlist.LogMsg('debug', 'starting ' + modname)
+            func(mlist, msg, msgdata)
+            mlist.LogMsg('debug', 'done with ' + modname)
+        except DiscardMessage:
+            # Throw the message away; we need do nothing else with it.
+            return 0
         except MessageHeld:
+            # Let the approval process take it from here.  The message no
+            # longer needs to be queued.
+            return 0
+        except SomeRecipientsFailed:
+            # The delivery module being used (SMTPDirect or Sendmail) failed
+            # to deliver the message to one or all of the recipients.  Push
+            # the delivery module back on the pipeline list and break.
+            pipeline.insert(0, modname)
+            # Consult and adjust some meager metrics that try to decide
+            # whether it's worth continuing to attempt delivery of this
+            # message.
+            now = time.time()
+            recips = msgdata['recips']
+            last_recip_count = msgdata.get('last_recip_count', 0)
+            deliver_until = msgdata.get('deliver_until', now)
+            if len(recips) == last_recip_count:
+                # We didn't make any progress.  How many times to we continue
+                # to attempt delivery?  TBD: make this configurable.
+                if now > deliver_until:
+                    # throw this message away
+                    return 0
+            else:
+                # Keep trying to delivery this for 3 days
+                deliver_until = now + 60*60*24*3
+            msgdata['last_recip_count'] = len(recips)
+            msgdata['deliver_until'] = deliver_until
             break
+        except Exception, e:
+            # Some other exception occurred, which we definitely did not
+            # expect, so set this message up for queuing.  This is mildly
+            # offensive since we're doing the equivalent of a bare except,
+            # which gobbles useful bug reporting.  Still, it's more important
+            # that email not get lost, so we log the exception and the
+            # traceback so that we have a hope of fixing this.  We may want to
+            # email the site admin or (shudder) the Mailman maintainers.
+            #
+            # We stick the name of the failed module back into the front of
+            # the pipeline list so that it can resume where it left off when
+            # qrunner tries to redeliver it.
+            pipeline.insert(0, modname)
+            mlist.LogMsg('error', 'Delivery exception: %s' % e)
+            s = StringIO()
+            traceback.print_exc(file=s)
+            mlist.LogMsg('error', s.getvalue())
+            break
+    msgdata['pipeline'] = pipeline
+    return len(pipeline)
 
 
 
-# for messages that arrive from the outside, to be delivered to all mailing
-# list subscribers
-def DeliverToList(mlist, msg):
-    pipeline = ['SpamDetect',
-                'Approve',
-                'Replybot',
-                'Hold',
-                'Cleanse',
-                'CookHeaders',
-                'ToDigest',
-                'ToArchive',
-                'ToUsenet',
-                'CalcRecips',
-                'Decorate',
-                mm_cfg.DELIVERY_MODULE,
-                'Acknowledge',
-                'AfterDelivery',
-                ]
-    pipeline_delivery(mlist, msg, pipeline)
-
-
-
-# for messages that qrunner tries to re-deliver
+# For messages that qrunner tries to re-deliver using the pre 2.0beta3 qfiles
+# data format.
 def RedeliverMessage(mlist, msg):
-    pipeline = [mm_cfg.DELIVERY_MODULE,
-                ]
-    pipeline_delivery(mlist, msg, pipeline)
+    msgdata = {'pipeline': [mm_cfg.DELIVERY_MODULE]}
+    return DeliverToList(mlist, msg, msgdata)
 
 
 
@@ -78,5 +143,8 @@ def DeliverToUser(mlist, msg):
                 'CookHeaders',
                 mm_cfg.DELIVERY_MODULE,
                 ]
-    msg.fasttrack = 1
-    pipeline_delivery(mlist, msg, pipeline)
+    msgdata = {'pipeline' : pipeline,
+               'fasttrack': 1,
+               'recips'   : msg.recips,
+               }
+    return DeliverToList(mlist, msg, msgdata)
diff --git a/Mailman/Handlers/Hold.py b/Mailman/Handlers/Hold.py
index 547cbd287..c896a5351 100644
--- a/Mailman/Handlers/Hold.py
+++ b/Mailman/Handlers/Hold.py
@@ -31,6 +31,7 @@ message handling should stop.
 import os
 import string
 import time
+from types import ClassType
 
 try:
     import cPickle
@@ -77,13 +78,17 @@ class SuspiciousHeaders(HandlerAPI.MessageHeld):
     pass
 
 class MessageTooBig(HandlerAPI.MessageHeld):
-    "Message body is too big"
-    pass
+    "Message body is too big: %d KB"
+    def __init__(self, msgsize):
+        self.__msgsize = msgsize
+
+    def __str__(self):
+        return HandlerAPI.Message.__str__(self) % self.__msgsize
 
 
 
-def process(mlist, msg):
-    if getattr(msg, 'approved', 0):
+def process(mlist, msg, msgdata):
+    if msgdata.get('approved'):
         return
     # get the sender of the message
     listname = mlist.internal_name()
@@ -101,7 +106,7 @@ def process(mlist, msg):
         forbiddens = Utils.List2Dict(mlist.forbidden_posters)
         addrs = Utils.FindMatchingAddresses(sender, forbiddens)
         if addrs:
-            hold_for_approval(mlist, msg, ForbiddenPoster)
+            hold_for_approval(mlist, msg, msgdata, ForbiddenPoster)
             # no return
     #
     # is the list moderated?  if so and the sender is not in the list of
@@ -110,7 +115,7 @@ def process(mlist, msg):
         posters = Utils.List2Dict(mlist.posters)
         addrs = Utils.FindMatchingAddresses(sender, posters)
         if not addrs:
-            hold_for_approval(mlist, msg, ModeratedPost)
+            hold_for_approval(mlist, msg, msgdata, ModeratedPost)
             # no return
     #
     # postings only from list members?  mlist.posters are allowed in addition
@@ -122,14 +127,14 @@ def process(mlist, msg):
            not Utils.FindMatchingAddresses(sender, posters):
             # the sender is neither a member of the list, nor in the list of
             # explicitly approved posters
-            hold_for_approval(mlist, msg, NonMemberPost)
+            hold_for_approval(mlist, msg, msgdata, NonMemberPost)
             # no return
     elif mlist.posters:
         posters = Utils.List2Dict(map(string.lower, mlist.posters))
         if not Utils.FindMatchingAddresses(sender, posters):
             # the sender is not explicitly in the list of allowed posters
             # (which is non-empty), so hold the message
-            hold_for_approval(mlist, msg, NotExplicitlyAllowed)
+            hold_for_approval(mlist, msg, msgdata, NotExplicitlyAllowed)
             # no return
     #
     # are there too many recipients to the message?
@@ -143,7 +148,7 @@ def process(mlist, msg):
         if ccheader:
             recips = recips + map(string.strip, string.split(ccheader, ','))
         if len(recips) > mlist.max_num_recipients:
-            hold_for_approval(mlist, msg, TooManyRecipients)
+            hold_for_approval(mlist, msg, msgdata, TooManyRecipients)
             # no return
     #
     # implicit destination?  Note that message originating from the Usenet
@@ -152,12 +157,12 @@ def process(mlist, msg):
        not mlist.HasExplicitDest(msg) and \
        not getattr(msg, 'fromusenet', 0):
         # then
-        hold_for_approval(mlist, msg, ImplicitDestination)
+        hold_for_approval(mlist, msg, msgdata, ImplicitDestination)
         # no return
     #
     # possible administrivia?
     if mlist.administrivia and Utils.IsAdministrivia(msg):
-        hold_for_approval(mlist, msg, Administrivia)
+        hold_for_approval(mlist, msg, msgdata, Administrivia)
         # no return
     #
     # suspicious headers?
@@ -166,28 +171,32 @@ def process(mlist, msg):
         if triggered:
             # TBD: Darn - can't include the matching line for the admin
             # message because the info would also go to the sender
-            hold_for_approval(mlist, msg, SuspiciousHeaders)
+            hold_for_approval(mlist, msg, msgdata, SuspiciousHeaders)
             # no return
     #
     # message too big?
     if mlist.max_message_size > 0:
-        if len(msg.body)/1024.0 > mlist.max_message_size:
-            hold_for_approval(mlist, msg, MessageTooBig)
+        bodylen = len(msg.body)/1024.0
+        if bodylen > mlist.max_message_size:
+            hold_for_approval(mlist, msg, msgdata, MessageTooBig(bodylen))
             # no return
 
 
 
-def hold_for_approval(mlist, msg, excclass):
+def hold_for_approval(mlist, msg, msgdata, exc):
     # TBD: This should really be tied into the email confirmation system so
     # that the message can be approved or denied via email as well as the
     # Web.  That's for later though, because it would mean a revamp of the
     # MailCommandHandler too.
     #
+    if type(exc) is ClassType:
+        # Go ahead and instantiate it now.
+        exc = exc()
     listname = mlist.real_name
-    reason = excclass.__doc__
+    reason = str(exc)
     sender = msg.GetSender()
     adminaddr = mlist.GetAdminEmail()
-    mlist.HoldMessage(msg, reason)
+    mlist.HoldMessage(msg, reason, msgdata)
     # now we need to craft and send a message to the list admin so they can
     # deal with the held message
     d = {'listname'   : listname,
@@ -216,4 +225,4 @@ def hold_for_approval(mlist, msg, excclass):
                  (listname, sender, reason))
     # raise the specific MessageHeld exception to exit out of the message
     # delivery pipeline
-    raise excclass
+    raise exc
diff --git a/Mailman/Handlers/Replybot.py b/Mailman/Handlers/Replybot.py
index 9869db906..174989100 100644
--- a/Mailman/Handlers/Replybot.py
+++ b/Mailman/Handlers/Replybot.py
@@ -26,35 +26,36 @@ from Mailman import Message
 
 
 
-def process(mlist, msg):
+def process(mlist, msg, msgdata):
     # "X-Ack: No" header in the original message disables the replybot
     ack = string.lower(msg.get('x-ack', ''))
     if ack == 'no':
         return
+    #
     # Check to see if the list is even configured to autorespond to this email
-    # message.  Note: the mailowner script sets the `toadmin' attribute, and
-    # the mailcmd script sets the `torequest' attribute.
-    toadmin = getattr(msg, 'toadmin', 0)
-    torequest = getattr(msg, 'torequest', 0)
-    if (toadmin and not mlist.autorespond_admin) or \
-       (torequest and not mlist.autorespond_requests) or \
-       (not toadmin and not torequest and not mlist.autorespond_postings):
+    # message.  Note: the mailowner script sets the `toadmin' key, and the
+    # mailcmd script sets the `torequest' key.
+    toadmin = msgdata.get('toadmin')
+    torequest = msgdata.get('torequest')
+    if ((toadmin and not mlist.autorespond_admin) or
+           (torequest and not mlist.autorespond_requests) or \
+           (not toadmin and not torequest and not mlist.autorespond_postings)):
         return
     #
-    # Now see if we're in the grace period for this sender (guaranteed to be
-    # lower cased).  graceperiod <= 0 means always autorespond, as does an
-    # "X-Ack: yes" header (useful for debugging).
+    # Now see if we're in the grace period for this sender.  graceperiod <= 0
+    # means always autorespond, as does an "X-Ack: yes" header (useful for
+    # debugging).
     sender = msg.GetSender()
     now = time.time()
     graceperiod = mlist.autoresponse_graceperiod
     if graceperiod > 0 and ack <> 'yes':
         if toadmin:
-            quite_until = mlist.admin_responses.get(sender, 0)
+            quiet_until = mlist.admin_responses.get(sender, 0)
         elif torequest:
-            quite_until = mlist.request_responses.get(sender, 0)
+            quiet_until = mlist.request_responses.get(sender, 0)
         else:
-            quite_until = mlist.postings_responses.get(sender, 0)
-        if quite_until > now:
+            quiet_until = mlist.postings_responses.get(sender, 0)
+        if quiet_until > now:
             return
     #
     # Okay, we know we're going to auto-respond to this sender, craft the
@@ -82,10 +83,10 @@ def process(mlist, msg):
     # update the grace period database
     if graceperiod > 0:
         # graceperiod is in days, we need # of seconds
-        quite_until = now + graceperiod * 24 * 60 * 60
+        quiet_until = now + graceperiod * 24 * 60 * 60
         if toadmin:
-            mlist.admin_responses[sender] = quite_until
+            mlist.admin_responses[sender] = quiet_until
         elif torequest:
-            mlist.request_responses[sender] = quite_until
+            mlist.request_responses[sender] = quiet_until
         else:
-            mlist.postings_responses[sender] = quite_until
+            mlist.postings_responses[sender] = quiet_until
diff --git a/Mailman/Handlers/SMTPDirect.py b/Mailman/Handlers/SMTPDirect.py
index 72ad9a770..7507338ab 100644
--- a/Mailman/Handlers/SMTPDirect.py
+++ b/Mailman/Handlers/SMTPDirect.py
@@ -26,8 +26,6 @@ isn't locked while delivery occurs synchronously.
 import os
 import time
 import socket
-import sha
-import marshal
 
 from Mailman import mm_cfg
 from Mailman import Utils
@@ -36,9 +34,10 @@ from Mailman.pythonlib import smtplib
 
 
 
-def process(mlist, msg):
-    if msg.recips == 0:
-        # nothing to do!
+def process(mlist, msg, msgdata):
+    recips = msgdata.get('recips')
+    if not recips:
+        # Nobody to deliver to!
         return
     # I want to record how long the SMTP dialog takes because this will help
     # diagnose whether we need to rewrite this module to relinquish the list
@@ -51,12 +50,12 @@ def process(mlist, msg):
             # make sure the connect happens, which won't be done by the
             # constructor if SMTPHOST is false
             envsender = mlist.GetAdminEmail()
-            refused = conn.sendmail(envsender, msg.recips, str(msg))
+            refused = conn.sendmail(envsender, recips, str(msg))
         finally:
             t1 = time.time()
             mlist.LogMsg('smtp',
                          'smtp for %d recips, completed in %.3f seconds' %
-                         (len(msg.recips), (t1-t0)))
+                         (len(recips), (t1-t0)))
             conn.quit()
     except smtplib.SMTPRecipientsRefused, e:
         refused = e.recipients
@@ -65,7 +64,8 @@ def process(mlist, msg):
     except (socket.error, smtplib.SMTPException), e:
         mlist.LogMsg('smtp', 'All recipients refused: %s' % e)
         # no recipients ever received the message
-        queue_message(mlist, msg)
+        msgdata['recips'] = recips
+        raise HandlerAPI.SomeRecipientsFailed
     #
     # Go through all refused recipients and deal with them if possible
     tempfailures = []
@@ -90,47 +90,5 @@ def process(mlist, msg):
             mlist.LogMsg('smtp-failure', '%d %s (%s)' % (code, recip, smtpmsg))
             tempfailures.append(recip)
     if tempfailures:
-        queue_message(mlist, msg, tempfailures)
-
-
-
-def queue_message(mlist, msg, recips=None):
-    if recips is None:
-        # i.e. total delivery failure
-        recips = msg.recips
-    # calculate a unique name for this file
-    text = str(msg)
-    filebase = sha.new(text).hexdigest()
-    msgfile = os.path.join(mm_cfg.QUEUE_DIR, filebase + '.msg')
-    dbfile = os.path.join(mm_cfg.QUEUE_DIR, filebase + '.db')
-    # Initialize the information about this message delivery.  It's possible a
-    # delivery attempt has been previously tried on this message, in which
-    # case, we'll just update the data.  We should probably do /some/ timing
-    # out of failed deliveries.
-    try:
-        dbfp = open(dbfile)
-        msgdata = marshal.load(dbfp)
-        dbfp.close()
-        msgdata['last_recip_count'] = len(msgdata['recips'])
-        msgdata['recips'] = recips
-        msgdata['attempts'] = msgdata['attempts'] + 1
-        existsp = 1
-    except (EOFError, ValueError, TypeError, IOError):
-        msgdata = {'listname'        : mlist.internal_name(),
-                   'recips'          : recips,
-                   'attempts'        : 1,
-                   'last_recip_count': -1,
-                   # any other stats we need?
-                   }
-        existsp = 0
-    # write the data file
-    dbfp = Utils.open_ex(dbfile, 'w')
-    marshal.dump(msgdata, dbfp)
-    dbfp.close()
-    # if it doesn't already exist, write the message file
-    if not existsp:
-        msgfp = Utils.open_ex(msgfile, 'w')
-        msgfp.write(text)
-        msgfp.close()
-    # this is a signal to qrunner
-    msg.failedcount = len(recips)
+        msgdata['recips'] = tempfailures
+        raise HandlerAPI.SomeRecipientsFailed
diff --git a/Mailman/Handlers/Sendmail.py b/Mailman/Handlers/Sendmail.py
index f24ed5627..6beebca9e 100644
--- a/Mailman/Handlers/Sendmail.py
+++ b/Mailman/Handlers/Sendmail.py
@@ -29,15 +29,11 @@ import os
 import HandlerAPI
 from Mailman import mm_cfg
 
-class SendmailHandlerError(HandlerAPI.HandlerError):
-    pass
-
-
 MAX_CMDLINE = 3000
 
 
 
-def process(mlist, msg):
+def process(mlist, msg, msgdata):
     """Process the message object for the given list.
 
     The message object is an instance of Mailman.Message and must be fully
@@ -52,8 +48,9 @@ def process(mlist, msg):
     program.
     
     """
-    if msg.recips == 0:
-        # nothing to do!
+    recips = msgdata.get('recips')
+    if not recips:
+        # Nobody to deliver to!
         return
     # Use -f to set the envelope sender
     cmd = mm_cfg.SENDMAIL_CMD + ' -f ' + mlist.GetAdminEmail() + ' '
@@ -61,7 +58,7 @@ def process(mlist, msg):
     recipchunks = []
     currentchunk = []
     chunklen = 0
-    for r in msg.recips:
+    for r in recips:
         currentchunk.append(r)
         chunklen = chunklen + len(r) + 1
         if chunklen > MAX_CMDLINE:
@@ -75,9 +72,10 @@ def process(mlist, msg):
     # over again
     msgtext = str(msg)
     # cycle through all chunks
-    for recips in recipchunks:
+    failedrecips = []
+    for chunk in recipchunks:
         # TBD: SECURITY ALERT.  This invokes the shell!
-        fp = os.popen(cmd + recips, 'w')
+        fp = os.popen(cmd + chunk, 'w')
         fp.write(msgtext)
         status = fp.close()
         if status:
@@ -85,7 +83,12 @@ def process(mlist, msg):
             mlist.LogMsg('post', 'post to %s from %s, size=%d, failure=%d' %
                          (mlist.internal_name(), msg.GetSender(),
                           len(msg.body), errcode))
-            raise SendmailHandlerError(errcode)
+            # TBD: can we do better than this?  What if only one recipient out
+            # of the entire chunk failed?
+            failedrecips.extend(chunk)
         # Log the successful post
         mlist.LogMsg('post', 'post to %s from %s, size=%d, success' %
                      (mlist.internal_name(), msg.GetSender(), len(msg.body)))
+    if failedrecips:
+        msgdata['recips'] = failedrecips
+        raise HandlerAPI.SomeRecipientsFailed
diff --git a/Mailman/Handlers/SpamDetect.py b/Mailman/Handlers/SpamDetect.py
index b4854816a..b8b6d49d8 100644
--- a/Mailman/Handlers/SpamDetect.py
+++ b/Mailman/Handlers/SpamDetect.py
@@ -18,15 +18,17 @@
 
 This module hard codes site wide spam detection.  By hacking the
 KNOWN_SPAMMERS variable, you can set up more regular expression matches
-against message headers.  If spam is detected, it is held for approval (see
-Hold.py).
+against message headers.  If spam is detected the message is discarded
+immediately.
 
 TBD: This needs to be made more configurable and robust.
 """
 
 import re
 import HandlerAPI
-import Hold
+
+class SpamDetected(HandlerAPI.DiscardMessage):
+    """The message contains known spam"""
 
 
 # This variable contains a list of 2-tuple of the format (header, regex) which
@@ -38,14 +40,9 @@ import Hold
 KNOWN_SPAMMERS = []
 
 
-class SpamDetected(HandlerAPI.MessageHeld):
-    """Potential spam detected"""
-    pass
-
-
 
-def process(mlist, msg):
-    if getattr(msg, 'approved', 0):
+def process(mlist, msg, msgdata):
+    if msgdata.get('approved'):
         return
     for header, regex in KNOWN_SPAMMERS:
         cre = re.compile(regex, re.IGNORECASE)
@@ -57,6 +54,5 @@ def process(mlist, msg):
                 continue
         mo = cre.search(text)
         if mo:
-            # we've detected spam
-            Hold.hold_for_approval(mlist, msg, SpamDetected)
-            # no return
+            # we've detected spam, so throw the message away
+            raise SpamDetected
diff --git a/Mailman/Handlers/ToArchive.py b/Mailman/Handlers/ToArchive.py
index 424bf305e..694c3b66e 100644
--- a/Mailman/Handlers/ToArchive.py
+++ b/Mailman/Handlers/ToArchive.py
@@ -17,13 +17,12 @@
 """Add the message to the archives."""
 
 import string
-from Mailman import mm_cfg
 
 
 
-def process(mlist, msg):
+def process(mlist, msg, msgdata):
     # short circuits
-    if getattr(msg, 'isdigest', 0) or not mlist.archive:
+    if msgdata.get('isdigest') or not mlist.archive:
         return
     archivep = msg.getheader('x-archive')
     if archivep and string.lower(archivep) == 'no':
diff --git a/Mailman/Handlers/ToDigest.py b/Mailman/Handlers/ToDigest.py
index 2e851a28b..e7ddd07d8 100644
--- a/Mailman/Handlers/ToDigest.py
+++ b/Mailman/Handlers/ToDigest.py
@@ -38,10 +38,10 @@ EXCLUDE_HEADERS = ('received', 'errors-to')
 
 
 
-def process(mlist, msg):
+def process(mlist, msg, msgdata):
     # short circuit non-digestable lists, or for messages that are already
     # digests
-    if not mlist.digestable or getattr(msg, 'isdigest', 0):
+    if not mlist.digestable or msgdata.get('isdigest'):
         return
     digestfile = os.path.join(mlist.fullpath(), 'next-digest')
     topicsfile = os.path.join(mlist.fullpath(), 'next-digest-topics')
@@ -109,7 +109,7 @@ def process(mlist, msg):
         size = os.stat(digestfile)[ST_SIZE]
         if size/1024.0 >= mlist.digest_size_threshhold:
             inject_digest(mlist, digestfile, topicsfile)
-    except os.error, e:
+    except OSError, e:
         code, msg = e
         if code == ENOENT:
             mlist.LogMsg('error', 'Lost digest file: %s' % digestfile)
@@ -155,20 +155,15 @@ def inject_digest(mlist, digestfile, topicsfile):
     # do any deliveries
     if mime_recips or text_recips:
         digest = Digest(mlist, topicsdata, fp.read())
-        # generate and post the MIME digest
+        # Generate the MIME digest, but only queue it for delivery so we don't
+        # hold the lock too long.
         msg = digest.asMIME()
         msg['To'] = mlist.GetListEmail()
-        msg.recips = mime_recips
-        msg.isdigest = 1
-        msg.approved = 1
-        mlist.Post(msg)
-        # generate and post the RFC934 "plain text" digest
+        msg.Enqueue(mlist, recips=mime_recips, isdigest=1, approved=1)
+        # Generate the RFC934 "plain text" digest, and again, just queue it
         msg = digest.asText()
-        msg.recips = text_recips
         msg['To'] = mlist.GetListEmail()
-        msg.isdigest = 1
-        msg.approved = 1
-        mlist.Post(msg)
+        msg.Enqueue(mlist, recips=text_recips, isdigest=1, approved=1)
     # zap accumulated digest information for the next round
     os.unlink(digestfile)
     os.unlink(topicsfile)
diff --git a/Mailman/Handlers/ToUsenet.py b/Mailman/Handlers/ToUsenet.py
index 48a3c4739..3700fdc99 100644
--- a/Mailman/Handlers/ToUsenet.py
+++ b/Mailman/Handlers/ToUsenet.py
@@ -25,12 +25,11 @@ import socket
 from Mailman.pythonlib.StringIO import StringIO
 
 
-def process(mlist, msg):
+def process(mlist, msg, msgdata):
     # short circuits
     if not mlist.gateway_to_news or \
-            getattr(msg, 'isdigest', 0) or \
-            getattr(msg, 'fromusenet', 0):
-        # then
+           msgdata.get('isdigest') or \
+           msgdata.get('fromusenet'):
         return
     # sanity checks
     error = []
@@ -39,17 +38,20 @@ def process(mlist, msg):
     if not mlist.nntp_host:
         error.append('no NNTP host')
     if error:
-        msg = 'NNTP gateway improperly configured: ' + string.join(error, ', ')
-        mlist.LogMsg('error', msg)
+        mlist.LogMsg('NNTP gateway improperly configured: ' +
+                     string.join(error, ', '))
         return
     # Fork in case the nntp connection hangs.
     pid = os.fork()
-    if not pid:
+    if pid:
+        # In the parent.  This is a bit of a kludge to keep a list of the
+        # children that need to be waited on.  We want to be sure to do the
+        # waiting while the list is unlocked!
+        kids = msgdata.get('kids', {})
+        kids[pid] = pid
+        msgdata['kids'] = kids
+    else:
         do_child(mlist, msg)
-    # TBD: we probably want to reap all those children, but do it in a way
-    # that doesn't keep the MailList object locked.  Problem is that we don't
-    # know what other handlers are going to execute.  Handling children should
-    # be pushed up into a higher module