7 files changed, 217 insertions, 76 deletions

diff --git a/src/mailman/bin/master.py b/src/mailman/bin/master.py
index d910b491d..e31780a31 100644
--- a/src/mailman/bin/master.py
+++ b/src/mailman/bin/master.py
@@ -175,7 +175,6 @@ def acquire_lock_1(force, lock_file=None):
         lock.disown()
         hostname, pid, tempfile = lock.details
         os.unlink(lock_file)
-        os.unlink(tempfile)
         return acquire_lock_1(force=False)
 
 
diff --git a/src/mailman/commands/cli_control.py b/src/mailman/commands/cli_control.py
index 2caaff54e..b2dc1bdd0 100644
--- a/src/mailman/commands/cli_control.py
+++ b/src/mailman/commands/cli_control.py
@@ -101,8 +101,10 @@ class Start:
         if status is WatcherState.conflict:
             self.parser.error(_('GNU Mailman is already running'))
         elif status in (WatcherState.stale_lock, WatcherState.host_mismatch):
-            self.parser.error(_('A previous run of GNU Mailman did not exit '
-                                'cleanly.  Try using --force.'))
+            if args.force is None:
+                self.parser.error(
+                    _('A previous run of GNU Mailman did not exit '
+                      'cleanly.  Try using --force.'))
         def log(message):
             if not args.quiet:
                 print message
diff --git a/src/mailman/commands/docs/control.rst b/src/mailman/commands/docs/control.rst
index 70b870668..184c5873b 100644
--- a/src/mailman/commands/docs/control.rst
+++ b/src/mailman/commands/docs/control.rst
@@ -13,37 +13,7 @@ All we care about is the master process; normally it starts a bunch of
 runners, but we don't care about any of them, so write a test configuration
 file for the master that disables all the runners.
 
-    >>> import shutil
-    >>> from os.path import dirname, join
-    >>> config_file = join(dirname(config.filename), 'no-runners.cfg')
-    >>> shutil.copyfile(config.filename, config_file)
-    >>> with open(config_file, 'a') as fp:
-    ...     print >> fp, """\
-    ... [runner.archive]
-    ... start: no
-    ... [runner.bounces]
-    ... start: no
-    ... [runner.command]
-    ... start: no
-    ... [runner.in]
-    ... start: no
-    ... [runner.lmtp]
-    ... start: no
-    ... [runner.news]
-    ... start: no
-    ... [runner.out]
-    ... start: no
-    ... [runner.pipeline]
-    ... start: no
-    ... [runner.rest]
-    ... start: no
-    ... [runner.retry]
-    ... start: no
-    ... [runner.virgin]
-    ... start: no
-    ... [runner.digest]
-    ... start: no
-    ... """
+    >>> from mailman.commands.tests.test_control import make_config
 
 
 Starting
@@ -56,7 +26,7 @@ Starting
     ...     force = False
     ...     run_as_user = True
     ...     quiet = False
-    ...     config = config_file
+    ...     config = make_config()
     >>> args = FakeArgs()
 
 Starting the daemons prints a useful message and starts the master watcher
@@ -65,34 +35,13 @@ process in the background.
     >>> start.process(args)
     Starting Mailman's master runner
 
-    >>> import errno, os, time
-    >>> from datetime import timedelta, datetime
-    >>> def find_master():
-    ...     until = timedelta(seconds=10) + datetime.now()
-    ...     while datetime.now() < until:
-    ...         time.sleep(0.1)
-    ...         try:
-    ...             with open(config.PID_FILE) as fp:
-    ...                 pid = int(fp.read().strip())
-    ...             os.kill(pid, 0)
-    ...         except IOError as error:
-    ...             if error.errno != errno.ENOENT:
-    ...                 raise
-    ...         except ValueError:
-    ...             pass
-    ...         except OSError as error:
-    ...             if error.errno != errno.ESRCH:
-    ...                 raise
-    ...         else:
-    ...             print 'Master process found'
-    ...             return pid
-    ...     else:
-    ...         raise AssertionError('No master process')
+    >>> from mailman.commands.tests.test_control import find_master
 
 The process exists, and its pid is available in a run time file.
 
     >>> pid = find_master()
-    Master process found
+    >>> pid is not None
+    True
 
 
 Stopping
@@ -100,18 +49,22 @@ Stopping
 
 You can also stop the master watcher process from the command line, which
 stops all the child processes too.
+::
 
     >>> from mailman.commands.cli_control import Stop
     >>> stop = Stop()
     >>> stop.process(args)
     Shutting down Mailman's master runner
 
+    >>> from datetime import datetime, timedelta
+    >>> import os
+    >>> import time
+    >>> import errno
     >>> def bury_master():
-    ...     until = timedelta(seconds=10) + datetime.now()
+    ...     until = timedelta(seconds=2) + datetime.now()
     ...     while datetime.now() < until:
     ...         time.sleep(0.1)
     ...         try:
-    ...             import sys
     ...             os.kill(pid, 0)
     ...             os.waitpid(pid, os.WNOHANG)
     ...         except OSError as error:
diff --git a/src/mailman/commands/tests/test_control.py b/src/mailman/commands/tests/test_control.py
new file mode 100644
index 000000000..105ba95d5
--- /dev/null
+++ b/src/mailman/commands/tests/test_control.py
@@ -0,0 +1,171 @@
+# Copyright (C) 2011 by the Free Software Foundation, Inc.
+#
+# This file is part of GNU Mailman.
+#
+# GNU Mailman is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free
+# Software Foundation, either version 3 of the License, or (at your option)
+# any later version.
+#
+# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along with
+# GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Test some additional corner cases for starting/stopping."""
+
+from __future__ import absolute_import, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'find_master',
+    'make_config',
+    'test_suite',
+    ]
+
+
+import os
+import sys
+import time
+import errno
+import signal
+import shutil
+import socket
+import unittest
+
+from datetime import timedelta, datetime
+
+from mailman.commands.cli_control import Start, kill_watcher
+from mailman.config import config
+from mailman.testing.layers import ConfigLayer
+
+SEP = '|'
+
+
+
+def make_config():
+    # All we care about is the master process; normally it starts a bunch of
+    # runners, but we don't care about any of them, so write a test
+    # configuration file for the master that disables all the runners.
+    new_config = 'no-runners.cfg'
+    config_file = os.path.join(os.path.dirname(config.filename), new_config)
+    shutil.copyfile(config.filename, config_file)
+    with open(config_file, 'a') as fp:
+        for runner_config in config.runner_configs:
+            print >> fp, '[{0}]\nstart:no\n'.format(runner_config.name)
+    return config_file
+
+
+def find_master():
+    # See if the master process is still running.
+    until = timedelta(seconds=10) + datetime.now()
+    while datetime.now() < until:
+        time.sleep(0.1)
+        try:
+            with open(config.PID_FILE) as fp:
+                pid = int(fp.read().strip())
+                os.kill(pid, 0)
+        except IOError as error:
+            if error.errno != errno.ENOENT:
+                raise
+        except ValueError:
+            pass
+        except OSError as error:
+            if error.errno != errno.ESRCH:
+                raise
+        else:
+            return pid
+    else:
+        return None
+
+
+
+class FakeArgs:
+    force = None
+    run_as_user = None
+    quiet = True
+    config = None
+
+
+class FakeParser:
+    def __init__(self):
+        self.message = None
+
+    def error(self, message):
+        self.message = message
+        sys.exit(1)
+
+
+
+class TestStart(unittest.TestCase):
+    """Test various starting scenarios."""
+
+    layer = ConfigLayer
+
+    def setUp(self):
+        self.command = Start()
+        self.command.parser = FakeParser()
+        self.args = FakeArgs()
+        self.args.config = make_config()
+
+    def tearDown(self):
+        try:
+            with open(config.PID_FILE) as fp:
+                master_pid = int(fp.read())
+        except OSError as error:
+            if error.errno != errno.ENOENT:
+                raise
+            # There is no master, so just ignore this.
+            return
+        kill_watcher(signal.SIGTERM)
+        os.waitpid(master_pid, 0)
+
+    def test_force_stale_lock(self):
+        # Fake an acquisition of the master lock by another process, which
+        # subsequently goes stale.  Start by finding a free process id.  Yes,
+        # this could race, but given that we're starting with our own PID and
+        # searching downward, it's less likely.
+        fake_pid = os.getpid() - 1
+        while fake_pid > 1:
+            try:
+                os.kill(fake_pid, 0)
+            except OSError as error:
+                if error.errno == errno.ESRCH:
+                    break
+            fake_pid -= 1
+        else:
+            raise RuntimeError('Cannot find free PID')
+        # Lock acquisition logic taken from flufl.lock.
+        claim_file = SEP.join((
+            config.LOCK_FILE,
+            socket.getfqdn(),
+            str(fake_pid),
+            '0'))
+        with open(config.LOCK_FILE, 'w') as fp:
+            fp.write(claim_file)
+        os.link(config.LOCK_FILE, claim_file)
+        expiration_date = datetime.now() - timedelta(minutes=2)
+        t = time.mktime(expiration_date.timetuple())
+        os.utime(claim_file, (t, t))
+        # Start without --force; no master will be running.
+        try:
+            self.command.process(self.args)
+        except SystemExit:
+            pass
+        self.assertEqual(find_master(), None)
+        self.assertTrue('--force' in self.command.parser.message)
+        # Start again, this time with --force.
+        self.args.force = True
+        self.command.process(self.args)
+        pid = find_master()
+        self.assertNotEqual(pid, None)
+
+
+
+def test_suite():
+    suite = unittest.TestSuite()
+    suite.addTest(unittest.makeSuite(TestStart))
+    return suite
diff --git a/src/mailman/commands/tests/test_create.py b/src/mailman/commands/tests/test_create.py
index c2176a106..76a8e0e00 100644
--- a/src/mailman/commands/tests/test_create.py
+++ b/src/mailman/commands/tests/test_create.py
@@ -25,6 +25,7 @@ __all__ = [
     ]
 
 
+import sys
 import unittest
 
 from mailman.app.lifecycle import create_list
@@ -48,6 +49,7 @@ class FakeParser:
 
     def error(self, message):
         self.message = message
+        sys.exit(1)
 
 
 
@@ -65,14 +67,20 @@ class TestCreate(unittest.TestCase):
         # Cannot create a mailing list if it already exists.
         create_list('test@example.com')
         self.args.listname = ['test@example.com']
-        self.command.process(self.args)
+        try:
+            self.command.process(self.args)
+        except SystemExit:
+            pass
         self.assertEqual(self.command.parser.message,
                          'List already exists: test@example.com')
 
     def test_invalid_posting_address(self):
         # Cannot create a mailing list with an invalid posting address.
         self.args.listname = ['foo']
-        self.command.process(self.args)
+        try:
+            self.command.process(self.args)
+        except SystemExit:
+            pass
         self.assertEqual(self.command.parser.message,
                          'Illegal list name: foo')
 
@@ -81,7 +89,10 @@ class TestCreate(unittest.TestCase):
         self.args.listname = ['test@example.com']
         self.args.domain = True
         self.args.owners = ['main=True']
-        self.command.process(self.args)
+        try:
+            self.command.process(self.args)
+        except SystemExit:
+            pass
         self.assertEqual(self.command.parser.message,
                          'Illegal owner addresses: main=True')
 
diff --git a/src/mailman/core/logging.py b/src/mailman/core/logging.py
index 09efd7771..fcd20b787 100644
--- a/src/mailman/core/logging.py
+++ b/src/mailman/core/logging.py
@@ -119,28 +119,32 @@ def initialize(propagate=None):
                         datefmt=config.logging.root.datefmt,
                         level=as_log_level(config.logging.root.level),
                         stream=sys.stderr)
-    # Create the subloggers.
+    # Create the sub-loggers.  Note that we'll redirect flufl.lock to
+    # mailman.locks.
     for logger_config in config.logger_configs:
         sub_name = logger_config.name.split('.')[-1]
         if sub_name == 'root':
             continue
-        logger_name = 'mailman.' + sub_name
-        log = logging.getLogger(logger_name)
+        if sub_name == 'locks':
+            log = logging.getLogger('flufl.lock')
+        else:
+            logger_name = 'mailman.' + sub_name
+            log = logging.getLogger(logger_name)
         # Get settings from log configuration file (or defaults).
-        log_format      = logger_config.format
-        log_datefmt     = logger_config.datefmt
+        log_format = logger_config.format
+        log_datefmt = logger_config.datefmt
         # Propagation to the root logger is how we handle logging to stderr
         # when the runners are not run as a subprocess of 'bin/mailman start'.
-        log.propagate   = (as_boolean(logger_config.propagate)
-                           if propagate is None else propagate)
+        log.propagate = (as_boolean(logger_config.propagate)
+                         if propagate is None else propagate)
         # Set the logger's level.
         log.setLevel(as_log_level(logger_config.level))
         # Create a formatter for this logger, then a handler, and link the
         # formatter to the handler.
         formatter = logging.Formatter(fmt=log_format, datefmt=log_datefmt)
-        path_str  = logger_config.path
-        path_abs  = os.path.normpath(os.path.join(config.LOG_DIR, path_str))
-        handler   = ReopenableFileHandler(sub_name, path_abs)
+        path_str = logger_config.path
+        path_abs = os.path.normpath(os.path.join(config.LOG_DIR, path_str))
+        handler = ReopenableFileHandler(sub_name, path_abs)
         _handlers[sub_name] = handler
         handler.setFormatter(formatter)
         log.addHandler(handler)
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index 45939b416..5b5518645 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -76,10 +76,11 @@ REST
 
 Commands
 --------
- * bin/qrunner is renamed to bin/runner.
- * `bin/mailman aliases` gains -f and -s options.
+ * `bin/qrunner` is renamed to `bin/runner`.
+ * `bin/mailman aliases` gains `-f` and `-s` options.
  * `bin/mailman create` no longer allows a list to be created with bogus owner
    addresses.  (LP: #778687)
+ * `bin/mailman start --force` option is fixed.  (LP: #869317)
 
 Documentation
 -------------