Merge branch 'invalid-pidfile' into delete-invalid-pidfile--lpirl

Conflicts:
	src/allmydata/scripts/startstop_node.py
	src/allmydata/test/cli/test_cli.py

src/allmydata/scripts/tahoe_daemonize.py

@@ -10,6 +10,35 @@ from allmydata.util.encodingutil import listdir_unicode, quote_local_unicode_pat
 from twisted.application.service import Service
 
 
+def get_pidfile(basedir):
+    """
+    Returns the path to the PID file.
+    :param basedir: the node's base directory
+    :returns: the path to the PID file
+    """
+    return os.path.join(basedir, u"twistd.pid")
+
+def get_pid_from_pidfile(pidfile):
+    """
+    Tries to read and return the PID stored in the node's PID file
+    (twistd.pid).
+    :param pidfile: try to read this PID file
+    :returns: A numeric PID on success, ``None`` if PID file absent or
+              inaccessible, ``-1`` if PID file invalid.
+    """
+    try:
+        with open(pidfile, "r") as f:
+            pid = f.read()
+    except EnvironmentError:
+        return None
+
+    try:
+        pid = int(pid)
+    except ValueError:
+        return -1
+
+    return pid
+
 def identify_node_type(basedir):
     """
     :return unicode: None or one of: 'client', 'introducer',
@@ -155,6 +184,12 @@ def daemonize(config):
         return 1
     twistd_config.loadedPlugins = {"DaemonizeTahoeNode": DaemonizeTahoeNodePlugin(nodetype, basedir)}
 
+    # handle invalid PID file (twistd might not start otherwise)
+    pidfile = get_pidfile(basedir)
+    if get_pid_from_pidfile(pidfile) == -1:
+        print >>err, "found invalid PID file in %s - deleting it" % basedir
+        os.remove(pidfile)
+
     # On Unix-like platforms:
     #   Unless --nodaemon was provided, the twistd.runApp() below spawns off a
     #   child process, and the parent calls os._exit(0), so there's no way for

src/allmydata/scripts/tahoe_stop.py

@@ -4,6 +4,7 @@ import signal
 
 from allmydata.scripts.common import BasedirOptions
 from allmydata.util.encodingutil import quote_local_unicode_path
+from .tahoe_daemonize import get_pidfile, get_pid_from_pidfile
 
 COULD_NOT_STOP = 2
 
@@ -23,21 +24,15 @@ def stop(config):
     basedir = config['basedir']
     quoted_basedir = quote_local_unicode_path(basedir)
     print >>out, "STOPPING", quoted_basedir
-    pidfile = os.path.join(basedir, u"twistd.pid")
+    pidfile = get_pidfile(basedir)
-    if not os.path.exists(pidfile):
+    pid = get_pid_from_pidfile(pidfile)
+    if pid is None:
         print >>err, "%s does not look like a running node directory (no twistd.pid)" % quoted_basedir
         # we define rc=2 to mean "nothing is running, but it wasn't me who
         # stopped it"
-        return COULD_NOT_STOP
+        return 2
-    with open(pidfile, "r") as f:
+    elif pid == -1:
-        pid = f.read()
+        print >>err, "%s contains an invalid PID file" % basedir
-
-    try:
-        pid = int(pid)
-    except ValueError:
-        # The error message below mimics a Twisted error message, which is
-        # displayed when starting a node with an invalid pidfile.
-        print >>err, "Pidfile %s contains non-numeric value" % pidfile
         # we define rc=2 to mean "nothing is running, but it wasn't me who
         # stopped it"
         return 2

src/allmydata/test/cli/test_cli.py

@@ -3,6 +3,7 @@ import os.path
 from cStringIO import StringIO
 import urllib, sys
 import re
+from mock import patch
 
 from twisted.trial import unittest
 from twisted.python.monkey import MonkeyPatcher
@@ -1312,4 +1313,31 @@ class Stop(unittest.TestCase):
 
         result_code = tahoe_stop.stop(config)
         self.assertEqual(2, result_code)
-        self.assertIn("contains non-numeric value", config.stderr.getvalue())
+        self.assertIn("invalid PID file", config.stderr.getvalue())
+
+
+class Start(unittest.TestCase):
+
+    @patch('allmydata.scripts.tahoe_daemonize.os.chdir')
+    @patch('allmydata.scripts.tahoe_daemonize.twistd')
+    def test_non_numeric_pid(self, mock_twistd, chdir):
+        """
+        If the pidfile exists but does not contain a numeric value, a complaint to
+        this effect is written to stderr.
+        """
+        basedir = FilePath(self.mktemp().decode("ascii"))
+        basedir.makedirs()
+        basedir.child(u"twistd.pid").setContent(b"foo")
+        basedir.child(u"tahoe-client.tac").setContent(b"")
+
+        config = tahoe_daemonize.DaemonizeOptions()
+        config.stdout = StringIO()
+        config.stderr = StringIO()
+        config['basedir'] = basedir.path
+        config.twistd_args = []
+
+        result_code = tahoe_daemonize.daemonize(config)
+        self.assertIn("invalid PID file", config.stderr.getvalue())
+        self.assertTrue(len(mock_twistd.mock_calls), 1)
+        self.assertEqual(mock_twistd.mock_calls[0][0], 'runApp')
+        self.assertEqual(0, result_code)