diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py index 21041f1ab..07f5bf72c 100644 --- a/src/allmydata/scripts/tahoe_run.py +++ b/src/allmydata/scripts/tahoe_run.py @@ -30,8 +30,8 @@ from allmydata.util.configutil import UnknownConfigError from allmydata.util.deferredutil import HookMixin from allmydata.util.pid import ( check_pid_process, - cleanup_pidfile, ProcessInTheWay, + InvalidPidFile, ) from allmydata.storage.crawler import ( MigratePickleFileError, @@ -237,8 +237,13 @@ def run(config, runApp=twistd.runApp): print("%s is not a recognizable node directory" % quoted_basedir, file=err) return 1 - # we turn off Twisted's pid-file to use our own - twistd_args = ["--pidfile", None, "--nodaemon", "--rundir", basedir] + twistd_args = [ + # turn off Twisted's pid-file to use our own + "--pidfile", None, + # ensure twistd machinery does not daemonize. + "--nodaemon", + "--rundir", basedir, + ] twistd_args.extend(config.twistd_args) twistd_args.append("DaemonizeTahoeNode") # point at our DaemonizeTahoeNodePlugin @@ -254,9 +259,7 @@ def run(config, runApp=twistd.runApp): return 1 twistd_config.loadedPlugins = {"DaemonizeTahoeNode": DaemonizeTahoeNodePlugin(nodetype, basedir)} - # before we try to run, check against our pidfile -- this will - # raise an exception if there appears to be a running process "in - # the way" + # our own pid-style file contains PID and process creation time pidfile = FilePath(get_pidfile(config['basedir'])) try: check_pid_process(pidfile) diff --git a/src/allmydata/test/cli/test_run.py b/src/allmydata/test/cli/test_run.py index 28613e8c1..db01eb440 100644 --- a/src/allmydata/test/cli/test_run.py +++ b/src/allmydata/test/cli/test_run.py @@ -159,7 +159,7 @@ class RunTests(SyncTestCase): """ basedir = FilePath(self.mktemp()).asTextMode() basedir.makedirs() - basedir.child(u"twistd.pid").setContent(b"foo") + basedir.child(u"running.process").setContent(b"foo") basedir.child(u"tahoe-client.tac").setContent(b"") config = RunOptions() @@ -168,17 +168,19 @@ class RunTests(SyncTestCase): config['basedir'] = basedir.path config.twistd_args = [] - runs = [] - result_code = run(config, runApp=runs.append) + class DummyRunner: + runs = [] + _exitSignal = None + + def run(self): + self.runs.append(True) + + result_code = run(config, runner=DummyRunner()) self.assertThat( config.stderr.getvalue(), Contains("found invalid PID file in"), ) self.assertThat( - runs, - HasLength(1), - ) - self.assertThat( - result_code, - Equals(0), + DummyRunner.runs, + Equals([]) ) diff --git a/src/allmydata/util/pid.py b/src/allmydata/util/pid.py index 21e30aa87..3b488a2c2 100644 --- a/src/allmydata/util/pid.py +++ b/src/allmydata/util/pid.py @@ -1,5 +1,8 @@ import os import psutil +from contextlib import ( + contextmanager, +) class ProcessInTheWay(Exception): @@ -8,6 +11,13 @@ class ProcessInTheWay(Exception): """ +class InvalidPidFile(Exception): + """ + our pidfile isn't well-formed + """ + + +@contextmanager def check_pid_process(pidfile, find_process=None): """ If another instance appears to be running already, raise an @@ -26,9 +36,16 @@ def check_pid_process(pidfile, find_process=None): if pidfile.exists(): with pidfile.open("r") as f: content = f.read().decode("utf8").strip() - pid, starttime = content.split() - pid = int(pid) - starttime = float(starttime) + try: + pid, starttime = content.split() + pid = int(pid) + starttime = float(starttime) + except ValueError: + raise InvalidPidFile( + "found invalid PID file in {}".format( + pidfile + ) + ) try: # if any other process is running at that PID, let the # user decide if this is another magic-older @@ -55,11 +72,7 @@ def check_pid_process(pidfile, find_process=None): with pidfile.open("w") as f: f.write("{} {}\n".format(pid, starttime).encode("utf8")) - -def cleanup_pidfile(pidfile): - """ - Safely remove the given pidfile - """ + yield # setup completed, await cleanup try: pidfile.remove()