From fb532a71ef4da28c91594e8d05695e267b747137 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 13 Sep 2022 22:43:09 -0600 Subject: [PATCH 01/44] own pid-file checks --- setup.py | 3 ++ src/allmydata/scripts/tahoe_run.py | 36 ++++++++++----- src/allmydata/util/pid.py | 72 ++++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 12 deletions(-) create mode 100644 src/allmydata/util/pid.py diff --git a/setup.py b/setup.py index c3ee4eb90..bd16a61ce 100644 --- a/setup.py +++ b/setup.py @@ -138,6 +138,9 @@ install_requires = [ "treq", "cbor2", "pycddl", + + # for pid-file support + "psutil", ] setup_requires = [ diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py index 51be32ee3..21041f1ab 100644 --- a/src/allmydata/scripts/tahoe_run.py +++ b/src/allmydata/scripts/tahoe_run.py @@ -19,6 +19,7 @@ import os, sys from allmydata.scripts.common import BasedirOptions from twisted.scripts import twistd from twisted.python import usage +from twisted.python.filepath import FilePath from twisted.python.reflect import namedAny from twisted.internet.defer import maybeDeferred from twisted.application.service import Service @@ -27,6 +28,11 @@ from allmydata.scripts.default_nodedir import _default_nodedir from allmydata.util.encodingutil import listdir_unicode, quote_local_unicode_path from allmydata.util.configutil import UnknownConfigError from allmydata.util.deferredutil import HookMixin +from allmydata.util.pid import ( + check_pid_process, + cleanup_pidfile, + ProcessInTheWay, +) from allmydata.storage.crawler import ( MigratePickleFileError, ) @@ -35,28 +41,31 @@ from allmydata.node import ( PrivacyError, ) + 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") + return os.path.join(basedir, u"running.process") + 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() + data = f.read().strip() except EnvironmentError: return None + pid, _ = data.split() try: pid = int(pid) except ValueError: @@ -64,6 +73,7 @@ def get_pid_from_pidfile(pidfile): return pid + def identify_node_type(basedir): """ :return unicode: None or one of: 'client' or 'introducer'. @@ -227,10 +237,8 @@ def run(config, runApp=twistd.runApp): print("%s is not a recognizable node directory" % quoted_basedir, file=err) return 1 - twistd_args = ["--nodaemon", "--rundir", basedir] - if sys.platform != "win32": - pidfile = get_pidfile(basedir) - twistd_args.extend(["--pidfile", pidfile]) + # we turn off Twisted's pid-file to use our own + twistd_args = ["--pidfile", None, "--nodaemon", "--rundir", basedir] twistd_args.extend(config.twistd_args) twistd_args.append("DaemonizeTahoeNode") # point at our DaemonizeTahoeNodePlugin @@ -246,12 +254,16 @@ def run(config, runApp=twistd.runApp): return 1 twistd_config.loadedPlugins = {"DaemonizeTahoeNode": DaemonizeTahoeNodePlugin(nodetype, basedir)} - # handle invalid PID file (twistd might not start otherwise) - if sys.platform != "win32" and get_pid_from_pidfile(pidfile) == -1: - print("found invalid PID file in %s - deleting it" % basedir, file=err) - os.remove(pidfile) + # 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" + pidfile = FilePath(get_pidfile(config['basedir'])) + try: + check_pid_process(pidfile) + except ProcessInTheWay as e: + print("ERROR: {}".format(e)) + return 1 # We always pass --nodaemon so twistd.runApp does not daemonize. - print("running node in %s" % (quoted_basedir,), file=out) runApp(twistd_config) return 0 diff --git a/src/allmydata/util/pid.py b/src/allmydata/util/pid.py new file mode 100644 index 000000000..21e30aa87 --- /dev/null +++ b/src/allmydata/util/pid.py @@ -0,0 +1,72 @@ +import os +import psutil + + +class ProcessInTheWay(Exception): + """ + our pidfile points at a running process + """ + + +def check_pid_process(pidfile, find_process=None): + """ + If another instance appears to be running already, raise an + exception. Otherwise, write our PID + start time to the pidfile + and arrange to delete it upon exit. + + :param FilePath pidfile: the file to read/write our PID from. + + :param Callable find_process: None, or a custom way to get a + Process objet (usually for tests) + + :raises ProcessInTheWay: if a running process exists at our PID + """ + find_process = psutil.Process if find_process is None else find_process + # check if we have another instance running already + 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: + # if any other process is running at that PID, let the + # user decide if this is another magic-older + # instance. Automated programs may use the start-time to + # help decide this (if the PID is merely recycled, the + # start-time won't match). + proc = find_process(pid) + raise ProcessInTheWay( + "A process is already running as PID {}".format(pid) + ) + except psutil.NoSuchProcess: + print( + "'{pidpath}' refers to {pid} that isn't running".format( + pidpath=pidfile.path, + pid=pid, + ) + ) + # nothing is running at that PID so it must be a stale file + pidfile.remove() + + # write our PID + start-time to the pid-file + pid = os.getpid() + starttime = find_process(pid).create_time() + with pidfile.open("w") as f: + f.write("{} {}\n".format(pid, starttime).encode("utf8")) + + +def cleanup_pidfile(pidfile): + """ + Safely remove the given pidfile + """ + + try: + pidfile.remove() + except Exception as e: + print( + "Couldn't remove '{pidfile}': {err}.".format( + pidfile=pidfile.path, + err=e, + ) + ) From 3bfb60c6f426cadc25bb201e6e59165cedd2b490 Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 15 Sep 2022 19:57:01 -0600 Subject: [PATCH 02/44] back to context-manager, simplify --- src/allmydata/scripts/tahoe_run.py | 15 +++++++++------ src/allmydata/test/cli/test_run.py | 20 +++++++++++--------- src/allmydata/util/pid.py | 29 +++++++++++++++++++++-------- 3 files changed, 41 insertions(+), 23 deletions(-) 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() From cad162bb8fb2d961c74f457be6e4495b00f0aeed Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 15 Sep 2022 19:59:18 -0600 Subject: [PATCH 03/44] should have pid-file on windows too, now --- src/allmydata/test/cli/test_run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/cli/test_run.py b/src/allmydata/test/cli/test_run.py index db01eb440..902e4011a 100644 --- a/src/allmydata/test/cli/test_run.py +++ b/src/allmydata/test/cli/test_run.py @@ -151,7 +151,7 @@ class RunTests(SyncTestCase): """ Tests for ``run``. """ - @skipIf(platform.isWindows(), "There are no PID files on Windows.") + def test_non_numeric_pid(self): """ If the pidfile exists but does not contain a numeric value, a complaint to From 0e0ebf6687280d0be5ae6a536a4f9d48958d03b7 Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 15 Sep 2022 20:06:32 -0600 Subject: [PATCH 04/44] more testing --- src/allmydata/test/cli/test_run.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/allmydata/test/cli/test_run.py b/src/allmydata/test/cli/test_run.py index 902e4011a..ecc81fe3f 100644 --- a/src/allmydata/test/cli/test_run.py +++ b/src/allmydata/test/cli/test_run.py @@ -20,6 +20,9 @@ from testtools import ( skipIf, ) +from hypothesis.strategies import text +from hypothesis import given + from testtools.matchers import ( Contains, Equals, @@ -44,6 +47,10 @@ from ...scripts.tahoe_run import ( RunOptions, run, ) +from ...util.pid import ( + check_pid_process, + InvalidPidFile, +) from ...scripts.runner import ( parse_options @@ -180,7 +187,18 @@ class RunTests(SyncTestCase): config.stderr.getvalue(), Contains("found invalid PID file in"), ) + # because the pidfile is invalid we shouldn't get to the + # .run() call itself. self.assertThat( DummyRunner.runs, Equals([]) ) + + @given(text()) + def test_pidfile_contents(self, content): + pidfile = FilePath("pidfile") + pidfile.setContent(content.encode("utf8")) + + with self.assertRaises(InvalidPidFile): + with check_pid_process(pidfile): + pass From e6adfc7726cc3e081d18b712e573ef265e49c3ca Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 15 Sep 2022 20:22:07 -0600 Subject: [PATCH 05/44] news --- newsfragments/3926.incompat | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 newsfragments/3926.incompat diff --git a/newsfragments/3926.incompat b/newsfragments/3926.incompat new file mode 100644 index 000000000..3f58b4ba8 --- /dev/null +++ b/newsfragments/3926.incompat @@ -0,0 +1,10 @@ +Record both the PID and the process creation-time + +A new kind of pidfile in `running.process` records both +the PID and the creation-time of the process. This facilitates +automatic discovery of a "stale" pidfile that points to a +currently-running process. If the recorded creation-time matches +the creation-time of the running process, then it is a still-running +`tahoe run` proecss. Otherwise, the file is stale. + +The `twistd.pid` file is no longer present. \ No newline at end of file From 6048d1d9a99e5f88cd423a9524bede823277709f Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 15 Sep 2022 21:13:30 -0600 Subject: [PATCH 06/44] in case hypothesis finds the magic --- src/allmydata/test/cli/test_run.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/allmydata/test/cli/test_run.py b/src/allmydata/test/cli/test_run.py index ecc81fe3f..7bf87eea9 100644 --- a/src/allmydata/test/cli/test_run.py +++ b/src/allmydata/test/cli/test_run.py @@ -12,6 +12,7 @@ from future.utils import PY2 if PY2: from future.builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +import re from six.moves import ( StringIO, ) @@ -21,7 +22,7 @@ from testtools import ( ) from hypothesis.strategies import text -from hypothesis import given +from hypothesis import given, assume from testtools.matchers import ( Contains, @@ -194,8 +195,11 @@ class RunTests(SyncTestCase): Equals([]) ) + good_file_content_re = re.compile(r"\w[0-9]*\w[0-9]*\w") + @given(text()) def test_pidfile_contents(self, content): + assume(not self.good_file_content_re.match(content)) pidfile = FilePath("pidfile") pidfile.setContent(content.encode("utf8")) From 642b604753dd9b9af2c740e04e65e58bbae00299 Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 15 Sep 2022 21:51:56 -0600 Subject: [PATCH 07/44] use stdin-closing for pidfile cleanup too --- src/allmydata/scripts/tahoe_run.py | 1 + src/allmydata/test/cli/test_run.py | 12 +++--------- src/allmydata/util/pid.py | 17 +++++++++++------ 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py index 07f5bf72c..20d5c2bf1 100644 --- a/src/allmydata/scripts/tahoe_run.py +++ b/src/allmydata/scripts/tahoe_run.py @@ -30,6 +30,7 @@ from allmydata.util.configutil import UnknownConfigError from allmydata.util.deferredutil import HookMixin from allmydata.util.pid import ( check_pid_process, + cleanup_pidfile, ProcessInTheWay, InvalidPidFile, ) diff --git a/src/allmydata/test/cli/test_run.py b/src/allmydata/test/cli/test_run.py index 7bf87eea9..71085fddd 100644 --- a/src/allmydata/test/cli/test_run.py +++ b/src/allmydata/test/cli/test_run.py @@ -176,14 +176,8 @@ class RunTests(SyncTestCase): config['basedir'] = basedir.path config.twistd_args = [] - class DummyRunner: - runs = [] - _exitSignal = None - - def run(self): - self.runs.append(True) - - result_code = run(config, runner=DummyRunner()) + runs = [] + result_code = run(config, runApp=runs.append) self.assertThat( config.stderr.getvalue(), Contains("found invalid PID file in"), @@ -191,7 +185,7 @@ class RunTests(SyncTestCase): # because the pidfile is invalid we shouldn't get to the # .run() call itself. self.assertThat( - DummyRunner.runs, + runs, Equals([]) ) diff --git a/src/allmydata/util/pid.py b/src/allmydata/util/pid.py index 3b488a2c2..3ab955cb3 100644 --- a/src/allmydata/util/pid.py +++ b/src/allmydata/util/pid.py @@ -1,8 +1,5 @@ import os import psutil -from contextlib import ( - contextmanager, -) class ProcessInTheWay(Exception): @@ -17,7 +14,12 @@ class InvalidPidFile(Exception): """ -@contextmanager +class CannotRemovePidFile(Exception): + """ + something went wrong removing the pidfile + """ + + def check_pid_process(pidfile, find_process=None): """ If another instance appears to be running already, raise an @@ -72,12 +74,15 @@ def check_pid_process(pidfile, find_process=None): with pidfile.open("w") as f: f.write("{} {}\n".format(pid, starttime).encode("utf8")) - yield # setup completed, await cleanup +def cleanup_pidfile(pidfile): + """ + Safely clean up a PID-file + """ try: pidfile.remove() except Exception as e: - print( + raise CannotRemovePidFile( "Couldn't remove '{pidfile}': {err}.".format( pidfile=pidfile.path, err=e, From 82c72ddede1dbbe97365877186af27928a996c0b Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 15 Sep 2022 21:58:20 -0600 Subject: [PATCH 08/44] cleanup --- src/allmydata/test/cli/test_run.py | 14 ++------------ src/allmydata/util/pid.py | 4 ++-- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/allmydata/test/cli/test_run.py b/src/allmydata/test/cli/test_run.py index 71085fddd..ae869e475 100644 --- a/src/allmydata/test/cli/test_run.py +++ b/src/allmydata/test/cli/test_run.py @@ -17,22 +17,14 @@ from six.moves import ( StringIO, ) -from testtools import ( - skipIf, -) - from hypothesis.strategies import text from hypothesis import given, assume from testtools.matchers import ( Contains, Equals, - HasLength, ) -from twisted.python.runtime import ( - platform, -) from twisted.python.filepath import ( FilePath, ) @@ -184,10 +176,8 @@ class RunTests(SyncTestCase): ) # because the pidfile is invalid we shouldn't get to the # .run() call itself. - self.assertThat( - runs, - Equals([]) - ) + self.assertThat(runs, Equals([])) + self.assertThat(result_code, Equals(1)) good_file_content_re = re.compile(r"\w[0-9]*\w[0-9]*\w") diff --git a/src/allmydata/util/pid.py b/src/allmydata/util/pid.py index 3ab955cb3..ff8129bbc 100644 --- a/src/allmydata/util/pid.py +++ b/src/allmydata/util/pid.py @@ -50,11 +50,11 @@ def check_pid_process(pidfile, find_process=None): ) try: # if any other process is running at that PID, let the - # user decide if this is another magic-older + # user decide if this is another legitimate # instance. Automated programs may use the start-time to # help decide this (if the PID is merely recycled, the # start-time won't match). - proc = find_process(pid) + find_process(pid) raise ProcessInTheWay( "A process is already running as PID {}".format(pid) ) From 228bbbc2fe791b83af0d495df44882a63456b59f Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 15 Sep 2022 22:39:59 -0600 Subject: [PATCH 09/44] new pid-file --- src/allmydata/test/cli_node_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/cli_node_api.py b/src/allmydata/test/cli_node_api.py index 410796be2..c324d5565 100644 --- a/src/allmydata/test/cli_node_api.py +++ b/src/allmydata/test/cli_node_api.py @@ -134,7 +134,7 @@ class CLINodeAPI(object): @property def twistd_pid_file(self): - return self.basedir.child(u"twistd.pid") + return self.basedir.child(u"running.process") @property def node_url_file(self): From 114d5e1ed8582fa130953227eced0528862ca381 Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 15 Sep 2022 23:08:46 -0600 Subject: [PATCH 10/44] pidfile on windows now --- src/allmydata/scripts/tahoe_run.py | 6 +++-- src/allmydata/test/test_runner.py | 36 ++++++++++++------------------ 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py index 20d5c2bf1..72b8e3eca 100644 --- a/src/allmydata/scripts/tahoe_run.py +++ b/src/allmydata/scripts/tahoe_run.py @@ -239,12 +239,14 @@ def run(config, runApp=twistd.runApp): return 1 twistd_args = [ - # turn off Twisted's pid-file to use our own - "--pidfile", None, # ensure twistd machinery does not daemonize. "--nodaemon", "--rundir", basedir, ] + if sys.platform != "win32": + # turn off Twisted's pid-file to use our own -- but only on + # windows, because twistd doesn't know about pidfiles there + twistd_args.extend(["--pidfile", None]) twistd_args.extend(config.twistd_args) twistd_args.append("DaemonizeTahoeNode") # point at our DaemonizeTahoeNodePlugin diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 3eb6b8a34..9b6357f46 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -418,9 +418,7 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin): tahoe.active() - # We don't keep track of PIDs in files on Windows. - if not platform.isWindows(): - self.assertTrue(tahoe.twistd_pid_file.exists()) + self.assertTrue(tahoe.twistd_pid_file.exists()) self.assertTrue(tahoe.node_url_file.exists()) # rm this so we can detect when the second incarnation is ready @@ -493,9 +491,7 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin): # change on restart storage_furl = fileutil.read(tahoe.storage_furl_file.path) - # We don't keep track of PIDs in files on Windows. - if not platform.isWindows(): - self.assertTrue(tahoe.twistd_pid_file.exists()) + self.assertTrue(tahoe.twistd_pid_file.exists()) # rm this so we can detect when the second incarnation is ready tahoe.node_url_file.remove() @@ -513,21 +509,18 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin): fileutil.read(tahoe.storage_furl_file.path), ) - if not platform.isWindows(): - self.assertTrue( - tahoe.twistd_pid_file.exists(), - "PID file ({}) didn't exist when we expected it to. " - "These exist: {}".format( - tahoe.twistd_pid_file, - tahoe.twistd_pid_file.parent().listdir(), - ), - ) + self.assertTrue( + tahoe.twistd_pid_file.exists(), + "PID file ({}) didn't exist when we expected it to. " + "These exist: {}".format( + tahoe.twistd_pid_file, + tahoe.twistd_pid_file.parent().listdir(), + ), + ) yield tahoe.stop_and_wait() - if not platform.isWindows(): - # twistd.pid should be gone by now. - self.assertFalse(tahoe.twistd_pid_file.exists()) - + # twistd.pid should be gone by now. + self.assertFalse(tahoe.twistd_pid_file.exists()) def _remove(self, res, file): fileutil.remove(file) @@ -610,9 +603,8 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin): ), ) - if not platform.isWindows(): - # It should not be running. - self.assertFalse(tahoe.twistd_pid_file.exists()) + # It should not be running. + self.assertFalse(tahoe.twistd_pid_file.exists()) # Wait for the operation to *complete*. If we got this far it's # because we got the expected message so we can expect the "tahoe ..." From aef2e96139fc0afc610736b181e218dce2aa9b79 Mon Sep 17 00:00:00 2001 From: meejah Date: Sat, 17 Sep 2022 16:28:25 -0600 Subject: [PATCH 11/44] refactor: dispatch with our reactor, pass to tahoe_run --- src/allmydata/scripts/runner.py | 12 ++++-------- src/allmydata/scripts/tahoe_run.py | 2 +- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/allmydata/scripts/runner.py b/src/allmydata/scripts/runner.py index a0d8a752b..756c26f2c 100644 --- a/src/allmydata/scripts/runner.py +++ b/src/allmydata/scripts/runner.py @@ -47,11 +47,6 @@ if _default_nodedir: NODEDIR_HELP += " [default for most commands: " + quote_local_unicode_path(_default_nodedir) + "]" -# XXX all this 'dispatch' stuff needs to be unified + fixed up -_control_node_dispatch = { - "run": tahoe_run.run, -} - process_control_commands = [ ("run", None, tahoe_run.RunOptions, "run a node without daemonizing"), ] # type: SubCommands @@ -195,6 +190,7 @@ def parse_or_exit(config, argv, stdout, stderr): return config def dispatch(config, + reactor, stdin=sys.stdin, stdout=sys.stdout, stderr=sys.stderr): command = config.subCommand so = config.subOptions @@ -206,8 +202,8 @@ def dispatch(config, if command in create_dispatch: f = create_dispatch[command] - elif command in _control_node_dispatch: - f = _control_node_dispatch[command] + elif command == "run": + f = lambda config: tahoe_run.run(reactor, config) elif command in debug.dispatch: f = debug.dispatch[command] elif command in admin.dispatch: @@ -361,7 +357,7 @@ def _run_with_reactor(reactor, config, argv, stdout, stderr): stderr, ) d.addCallback(_maybe_enable_eliot_logging, reactor) - d.addCallback(dispatch, stdout=stdout, stderr=stderr) + d.addCallback(dispatch, reactor, stdout=stdout, stderr=stderr) def _show_exception(f): # when task.react() notices a non-SystemExit exception, it does # log.err() with the failure and then exits with rc=1. We want this diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py index 72b8e3eca..dd4561a4b 100644 --- a/src/allmydata/scripts/tahoe_run.py +++ b/src/allmydata/scripts/tahoe_run.py @@ -217,7 +217,7 @@ class DaemonizeTahoeNodePlugin(object): return DaemonizeTheRealService(self.nodetype, self.basedir, so) -def run(config, runApp=twistd.runApp): +def run(reactor, config, runApp=twistd.runApp): """ Runs a Tahoe-LAFS node in the foreground. From 8b2cb79070edabd20fb9bdbb41de51458788e50a Mon Sep 17 00:00:00 2001 From: meejah Date: Sat, 17 Sep 2022 16:29:03 -0600 Subject: [PATCH 12/44] cleanup via reactor --- src/allmydata/scripts/tahoe_run.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py index dd4561a4b..a5b833233 100644 --- a/src/allmydata/scripts/tahoe_run.py +++ b/src/allmydata/scripts/tahoe_run.py @@ -269,6 +269,11 @@ def run(reactor, config, runApp=twistd.runApp): except ProcessInTheWay as e: print("ERROR: {}".format(e)) return 1 + else: + reactor.addSystemEventTrigger( + "during", "shutdown", + lambda: cleanup_pidfile(pidfile) + ) # We always pass --nodaemon so twistd.runApp does not daemonize. runApp(twistd_config) From 254a994eb53035b70a653b47b2951d6159634a23 Mon Sep 17 00:00:00 2001 From: meejah Date: Sat, 17 Sep 2022 16:41:17 -0600 Subject: [PATCH 13/44] flake8 --- src/allmydata/scripts/tahoe_run.py | 2 +- src/allmydata/test/test_runner.py | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py index a5b833233..7722fef51 100644 --- a/src/allmydata/scripts/tahoe_run.py +++ b/src/allmydata/scripts/tahoe_run.py @@ -266,7 +266,7 @@ def run(reactor, config, runApp=twistd.runApp): pidfile = FilePath(get_pidfile(config['basedir'])) try: check_pid_process(pidfile) - except ProcessInTheWay as e: + except (ProcessInTheWay, InvalidPidFile) as e: print("ERROR: {}".format(e)) return 1 else: diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 9b6357f46..14d0dfb7f 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -47,9 +47,6 @@ from twisted.internet.defer import ( DeferredList, ) from twisted.python.filepath import FilePath -from twisted.python.runtime import ( - platform, -) from allmydata.util import fileutil, pollmixin from allmydata.util.encodingutil import unicode_to_argv from allmydata.test import common_util From fe80126e3fcffe56c171188c7cd5847f19bf6f7b Mon Sep 17 00:00:00 2001 From: meejah Date: Sun, 18 Sep 2022 22:39:25 -0600 Subject: [PATCH 14/44] fixups --- src/allmydata/scripts/tahoe_run.py | 2 +- src/allmydata/test/cli/test_run.py | 4 +++- src/allmydata/test/common_util.py | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py index 7722fef51..6dfa726a3 100644 --- a/src/allmydata/scripts/tahoe_run.py +++ b/src/allmydata/scripts/tahoe_run.py @@ -267,7 +267,7 @@ def run(reactor, config, runApp=twistd.runApp): try: check_pid_process(pidfile) except (ProcessInTheWay, InvalidPidFile) as e: - print("ERROR: {}".format(e)) + print("ERROR: {}".format(e), file=err) return 1 else: reactor.addSystemEventTrigger( diff --git a/src/allmydata/test/cli/test_run.py b/src/allmydata/test/cli/test_run.py index ae869e475..6358b70dd 100644 --- a/src/allmydata/test/cli/test_run.py +++ b/src/allmydata/test/cli/test_run.py @@ -168,8 +168,10 @@ class RunTests(SyncTestCase): config['basedir'] = basedir.path config.twistd_args = [] + from twisted.internet import reactor + runs = [] - result_code = run(config, runApp=runs.append) + result_code = run(reactor, config, runApp=runs.append) self.assertThat( config.stderr.getvalue(), Contains("found invalid PID file in"), diff --git a/src/allmydata/test/common_util.py b/src/allmydata/test/common_util.py index e63c3eef8..b6d352ab1 100644 --- a/src/allmydata/test/common_util.py +++ b/src/allmydata/test/common_util.py @@ -145,6 +145,7 @@ def run_cli_native(verb, *args, **kwargs): ) d.addCallback( runner.dispatch, + reactor, stdin=stdin, stdout=stdout, stderr=stderr, From 81c8e1c57b8b926ebb3a396f653d7149bd4f6577 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 20 Sep 2022 14:24:02 -0600 Subject: [PATCH 15/44] windows is special --- src/allmydata/test/test_runner.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 14d0dfb7f..5d8143558 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -42,6 +42,7 @@ from twisted.trial import unittest from twisted.internet import reactor from twisted.python import usage +from twisted.python.runtime import platform from twisted.internet.defer import ( inlineCallbacks, DeferredList, @@ -516,8 +517,12 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin): ) yield tahoe.stop_and_wait() - # twistd.pid should be gone by now. - self.assertFalse(tahoe.twistd_pid_file.exists()) + # twistd.pid should be gone by now -- except on Windows, where + # killing a subprocess immediately exits with no chance for + # any shutdown code (that is, no Twisted shutdown hooks can + # run). + if not platform.isWindows(): + self.assertFalse(tahoe.twistd_pid_file.exists()) def _remove(self, res, file): fileutil.remove(file) From 6db1476dacc99c00a12d88b1c6af6a8aa76f3404 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 20 Sep 2022 14:44:21 -0600 Subject: [PATCH 16/44] comment typo --- src/allmydata/scripts/tahoe_run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py index 6dfa726a3..721ced376 100644 --- a/src/allmydata/scripts/tahoe_run.py +++ b/src/allmydata/scripts/tahoe_run.py @@ -244,7 +244,7 @@ def run(reactor, config, runApp=twistd.runApp): "--rundir", basedir, ] if sys.platform != "win32": - # turn off Twisted's pid-file to use our own -- but only on + # turn off Twisted's pid-file to use our own -- but not on # windows, because twistd doesn't know about pidfiles there twistd_args.extend(["--pidfile", None]) twistd_args.extend(config.twistd_args) From 0eeb11c9cd45598fbe2e5bdccb4f9cf50fe222f3 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 20 Sep 2022 14:44:51 -0600 Subject: [PATCH 17/44] after shutdown --- src/allmydata/scripts/tahoe_run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py index 721ced376..40c4a6612 100644 --- a/src/allmydata/scripts/tahoe_run.py +++ b/src/allmydata/scripts/tahoe_run.py @@ -271,7 +271,7 @@ def run(reactor, config, runApp=twistd.runApp): return 1 else: reactor.addSystemEventTrigger( - "during", "shutdown", + "after", "shutdown", lambda: cleanup_pidfile(pidfile) ) From 77bc83d341794afbd0c6884fb5e0e914dbe90632 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 20 Sep 2022 14:45:19 -0600 Subject: [PATCH 18/44] incorrectly removed --- src/allmydata/scripts/tahoe_run.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py index 40c4a6612..eb4bb0b66 100644 --- a/src/allmydata/scripts/tahoe_run.py +++ b/src/allmydata/scripts/tahoe_run.py @@ -276,5 +276,6 @@ def run(reactor, config, runApp=twistd.runApp): ) # We always pass --nodaemon so twistd.runApp does not daemonize. + print("running node in %s" % (quoted_basedir,), file=out) runApp(twistd_config) return 0 From 1f29cc9c29e42a472ce893259f5bdbf2a31c00e0 Mon Sep 17 00:00:00 2001 From: meejah Date: Tue, 20 Sep 2022 14:50:46 -0600 Subject: [PATCH 19/44] windows special --- src/allmydata/test/test_runner.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 5d8143558..cf6e9f3b5 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -605,8 +605,10 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin): ), ) - # It should not be running. - self.assertFalse(tahoe.twistd_pid_file.exists()) + # It should not be running (but windows shutdown can't run + # code so the PID file still exists there). + if not platform.isWindows(): + self.assertFalse(tahoe.twistd_pid_file.exists()) # Wait for the operation to *complete*. If we got this far it's # because we got the expected message so we can expect the "tahoe ..." From 5973196931d2143f68a34d9b01857339582ec5c0 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 21 Sep 2022 19:00:27 -0600 Subject: [PATCH 20/44] refactor: use filelock and test it --- setup.py | 1 + src/allmydata/test/test_runner.py | 47 ++++++++++++ src/allmydata/util/pid.py | 117 ++++++++++++++++++------------ 3 files changed, 119 insertions(+), 46 deletions(-) diff --git a/setup.py b/setup.py index bd16a61ce..d99831347 100644 --- a/setup.py +++ b/setup.py @@ -141,6 +141,7 @@ install_requires = [ # for pid-file support "psutil", + "filelock", ] setup_requires = [ diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index cf6e9f3b5..5a8311649 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -50,6 +50,11 @@ from twisted.internet.defer import ( from twisted.python.filepath import FilePath from allmydata.util import fileutil, pollmixin from allmydata.util.encodingutil import unicode_to_argv +from allmydata.util.pid import ( + check_pid_process, + _pidfile_to_lockpath, + ProcessInTheWay, +) from allmydata.test import common_util import allmydata from allmydata.scripts.runner import ( @@ -617,3 +622,45 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin): # What's left is a perfect indicator that the process has exited and # we won't get blamed for leaving the reactor dirty. yield client_running + + +class PidFileLocking(SyncTestCase): + """ + Direct tests for allmydata.util.pid functions + """ + + def test_locking(self): + """ + Fail to create a pidfile if another process has the lock already. + """ + # this can't just be "our" process because the locking library + # allows the same process to acquire a lock multiple times. + pidfile = FilePath("foo") + lockfile = _pidfile_to_lockpath(pidfile) + + with open("code.py", "w") as f: + f.write( + "\n".join([ + "import filelock, time", + "with filelock.FileLock('{}', timeout=1):".format(lockfile.path), + " print('.', flush=True)", + " time.sleep(5)", + ]) + ) + proc = Popen( + [sys.executable, "code.py"], + stdout=PIPE, + stderr=PIPE, + start_new_session=True, + ) + # make sure our subprocess has had time to acquire the lock + # for sure (from the "." it prints) + self.assertThat( + proc.stdout.read(1), + Equals(b".") + ) + + # we should not be able to acuire this corresponding lock as well + with self.assertRaises(ProcessInTheWay): + check_pid_process(pidfile) + proc.terminate() diff --git a/src/allmydata/util/pid.py b/src/allmydata/util/pid.py index ff8129bbc..e256615d6 100644 --- a/src/allmydata/util/pid.py +++ b/src/allmydata/util/pid.py @@ -1,6 +1,10 @@ import os import psutil +# the docs are a little misleading, but this is either WindowsFileLock +# or UnixFileLock depending upon the platform we're currently on +from filelock import FileLock, Timeout + class ProcessInTheWay(Exception): """ @@ -20,6 +24,14 @@ class CannotRemovePidFile(Exception): """ +def _pidfile_to_lockpath(pidfile): + """ + internal helper. + :returns FilePath: a path to use for file-locking the given pidfile + """ + return pidfile.sibling("{}.lock".format(pidfile.basename())) + + def check_pid_process(pidfile, find_process=None): """ If another instance appears to be running already, raise an @@ -34,57 +46,70 @@ def check_pid_process(pidfile, find_process=None): :raises ProcessInTheWay: if a running process exists at our PID """ find_process = psutil.Process if find_process is None else find_process - # check if we have another instance running already - if pidfile.exists(): - with pidfile.open("r") as f: - content = f.read().decode("utf8").strip() - 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 legitimate - # instance. Automated programs may use the start-time to - # help decide this (if the PID is merely recycled, the - # start-time won't match). - find_process(pid) - raise ProcessInTheWay( - "A process is already running as PID {}".format(pid) - ) - except psutil.NoSuchProcess: - print( - "'{pidpath}' refers to {pid} that isn't running".format( - pidpath=pidfile.path, - pid=pid, - ) - ) - # nothing is running at that PID so it must be a stale file - pidfile.remove() + lock_path = _pidfile_to_lockpath(pidfile) - # write our PID + start-time to the pid-file - pid = os.getpid() - starttime = find_process(pid).create_time() - with pidfile.open("w") as f: - f.write("{} {}\n".format(pid, starttime).encode("utf8")) + try: + # a short timeout is fine, this lock should only be active + # while someone is reading or deleting the pidfile .. and + # facilitates testing the locking itself. + with FileLock(lock_path.path, timeout=2): + # check if we have another instance running already + if pidfile.exists(): + with pidfile.open("r") as f: + content = f.read().decode("utf8").strip() + 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 legitimate + # instance. Automated programs may use the start-time to + # help decide this (if the PID is merely recycled, the + # start-time won't match). + find_process(pid) + raise ProcessInTheWay( + "A process is already running as PID {}".format(pid) + ) + except psutil.NoSuchProcess: + print( + "'{pidpath}' refers to {pid} that isn't running".format( + pidpath=pidfile.path, + pid=pid, + ) + ) + # nothing is running at that PID so it must be a stale file + pidfile.remove() + + # write our PID + start-time to the pid-file + pid = os.getpid() + starttime = find_process(pid).create_time() + with pidfile.open("w") as f: + f.write("{} {}\n".format(pid, starttime).encode("utf8")) + except Timeout: + raise ProcessInTheWay( + "Another process is still locking {}".format(pidfile.path) + ) def cleanup_pidfile(pidfile): """ Safely clean up a PID-file """ - try: - pidfile.remove() - except Exception as e: - raise CannotRemovePidFile( - "Couldn't remove '{pidfile}': {err}.".format( - pidfile=pidfile.path, - err=e, + lock_path = _pidfile_to_lockpath(pidfile) + with FileLock(lock_path.path): + try: + pidfile.remove() + except Exception as e: + raise CannotRemovePidFile( + "Couldn't remove '{pidfile}': {err}.".format( + pidfile=pidfile.path, + err=e, + ) ) - ) From ea39e4ca6902daad125596f5e1e2b81989e9cb6b Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 21 Sep 2022 19:01:28 -0600 Subject: [PATCH 21/44] docstring --- src/allmydata/test/cli/test_run.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/allmydata/test/cli/test_run.py b/src/allmydata/test/cli/test_run.py index 6358b70dd..551164d3c 100644 --- a/src/allmydata/test/cli/test_run.py +++ b/src/allmydata/test/cli/test_run.py @@ -185,6 +185,9 @@ class RunTests(SyncTestCase): @given(text()) def test_pidfile_contents(self, content): + """ + invalid contents for a pidfile raise errors + """ assume(not self.good_file_content_re.match(content)) pidfile = FilePath("pidfile") pidfile.setContent(content.encode("utf8")) From 56775dde192c90b48fa85cfcb4a2651f5b264791 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 21 Sep 2022 19:05:30 -0600 Subject: [PATCH 22/44] refactor: parsing in a function --- src/allmydata/scripts/tahoe_run.py | 6 +++--- src/allmydata/util/pid.py | 34 +++++++++++++++++++----------- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py index eb4bb0b66..4d17492d4 100644 --- a/src/allmydata/scripts/tahoe_run.py +++ b/src/allmydata/scripts/tahoe_run.py @@ -29,6 +29,7 @@ from allmydata.util.encodingutil import listdir_unicode, quote_local_unicode_pat from allmydata.util.configutil import UnknownConfigError from allmydata.util.deferredutil import HookMixin from allmydata.util.pid import ( + parse_pidfile, check_pid_process, cleanup_pidfile, ProcessInTheWay, @@ -66,10 +67,9 @@ def get_pid_from_pidfile(pidfile): except EnvironmentError: return None - pid, _ = data.split() try: - pid = int(pid) - except ValueError: + pid, _ = parse_pidfile(pidfile) + except InvalidPidFile: return -1 return pid diff --git a/src/allmydata/util/pid.py b/src/allmydata/util/pid.py index e256615d6..1cb2cc45a 100644 --- a/src/allmydata/util/pid.py +++ b/src/allmydata/util/pid.py @@ -32,6 +32,27 @@ def _pidfile_to_lockpath(pidfile): return pidfile.sibling("{}.lock".format(pidfile.basename())) +def parse_pidfile(pidfile): + """ + :param FilePath pidfile: + :returns tuple: 2-tuple of pid, creation-time as int, float + :raises InvalidPidFile: on error + """ + with pidfile.open("r") as f: + content = f.read().decode("utf8").strip() + try: + pid, starttime = content.split() + pid = int(pid) + starttime = float(starttime) + except ValueError: + raise InvalidPidFile( + "found invalid PID file in {}".format( + pidfile + ) + ) + return pid, startime + + def check_pid_process(pidfile, find_process=None): """ If another instance appears to be running already, raise an @@ -55,18 +76,7 @@ def check_pid_process(pidfile, find_process=None): with FileLock(lock_path.path, timeout=2): # check if we have another instance running already if pidfile.exists(): - with pidfile.open("r") as f: - content = f.read().decode("utf8").strip() - try: - pid, starttime = content.split() - pid = int(pid) - starttime = float(starttime) - except ValueError: - raise InvalidPidFile( - "found invalid PID file in {}".format( - pidfile - ) - ) + pid, starttime = parse_pidfile(pidfile) try: # if any other process is running at that PID, let the # user decide if this is another legitimate From 390c8c52da3f9f0ea21ad789caf7c8f6ae9bbc74 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 21 Sep 2022 19:23:30 -0600 Subject: [PATCH 23/44] formatting + typo --- newsfragments/3926.incompat | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/newsfragments/3926.incompat b/newsfragments/3926.incompat index 3f58b4ba8..674ad289c 100644 --- a/newsfragments/3926.incompat +++ b/newsfragments/3926.incompat @@ -1,10 +1,10 @@ -Record both the PID and the process creation-time +Record both the PID and the process creation-time: -A new kind of pidfile in `running.process` records both +a new kind of pidfile in `running.process` records both the PID and the creation-time of the process. This facilitates automatic discovery of a "stale" pidfile that points to a currently-running process. If the recorded creation-time matches the creation-time of the running process, then it is a still-running -`tahoe run` proecss. Otherwise, the file is stale. +`tahoe run` process. Otherwise, the file is stale. The `twistd.pid` file is no longer present. \ No newline at end of file From e111694b3e33e54db974acbd057d74380c6de4ce Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 21 Sep 2022 19:28:09 -0600 Subject: [PATCH 24/44] get rid of find_process= --- src/allmydata/util/pid.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/allmydata/util/pid.py b/src/allmydata/util/pid.py index 1cb2cc45a..d681d819e 100644 --- a/src/allmydata/util/pid.py +++ b/src/allmydata/util/pid.py @@ -53,7 +53,7 @@ def parse_pidfile(pidfile): return pid, startime -def check_pid_process(pidfile, find_process=None): +def check_pid_process(pidfile): """ If another instance appears to be running already, raise an exception. Otherwise, write our PID + start time to the pidfile @@ -61,12 +61,8 @@ def check_pid_process(pidfile, find_process=None): :param FilePath pidfile: the file to read/write our PID from. - :param Callable find_process: None, or a custom way to get a - Process objet (usually for tests) - :raises ProcessInTheWay: if a running process exists at our PID """ - find_process = psutil.Process if find_process is None else find_process lock_path = _pidfile_to_lockpath(pidfile) try: @@ -83,7 +79,7 @@ def check_pid_process(pidfile, find_process=None): # instance. Automated programs may use the start-time to # help decide this (if the PID is merely recycled, the # start-time won't match). - find_process(pid) + psutil.Process(pid) raise ProcessInTheWay( "A process is already running as PID {}".format(pid) ) @@ -98,8 +94,7 @@ def check_pid_process(pidfile, find_process=None): pidfile.remove() # write our PID + start-time to the pid-file - pid = os.getpid() - starttime = find_process(pid).create_time() + starttime = psutil.Process().create_time() with pidfile.open("w") as f: f.write("{} {}\n".format(pid, starttime).encode("utf8")) except Timeout: From 0a09d23525fc4be928fa288c1301327d6eaccf32 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 21 Sep 2022 19:29:40 -0600 Subject: [PATCH 25/44] more docstring --- src/allmydata/util/pid.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/allmydata/util/pid.py b/src/allmydata/util/pid.py index d681d819e..f965d72ab 100644 --- a/src/allmydata/util/pid.py +++ b/src/allmydata/util/pid.py @@ -105,7 +105,8 @@ def check_pid_process(pidfile): def cleanup_pidfile(pidfile): """ - Safely clean up a PID-file + Remove the pidfile specified (respecting locks). If anything at + all goes wrong, `CannotRemovePidFile` is raised. """ lock_path = _pidfile_to_lockpath(pidfile) with FileLock(lock_path.path): From 6eebbda7c6c06732932c50a96ba1a5315c9d35f4 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 21 Sep 2022 20:07:29 -0600 Subject: [PATCH 26/44] documentation, example code --- docs/check_running.py | 47 +++++++++++++++++++++++++++++++++++++++++++ docs/running.rst | 29 ++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 docs/check_running.py diff --git a/docs/check_running.py b/docs/check_running.py new file mode 100644 index 000000000..ecc55da34 --- /dev/null +++ b/docs/check_running.py @@ -0,0 +1,47 @@ + +import psutil +import filelock + + +def can_spawn_tahoe(pidfile): + """ + Determine if we can spawn a Tahoe-LAFS for the given pidfile. That + pidfile may be deleted if it is stale. + + :param pathlib.Path pidfile: the file to check, that is the Path + to "running.process" in a Tahoe-LAFS configuration directory + + :returns bool: True if we can spawn `tahoe run` here + """ + lockpath = pidfile.parent / (pidfile.name + ".lock") + with filelock.FileLock(lockpath): + try: + with pidfile.open("r") as f: + pid, create_time = f.read().strip().split(" ", 1) + except FileNotFoundError: + return True + + # somewhat interesting: we have a pidfile + pid = int(pid) + create_time = float(create_time) + + try: + proc = psutil.Process(pid) + # most interesting case: there _is_ a process running at the + # recorded PID -- but did it just happen to get that PID, or + # is it the very same one that wrote the file? + if create_time == proc.create_time(): + # _not_ stale! another intance is still running against + # this configuration + return False + + except psutil.NoSuchProcess: + pass + + # the file is stale + pidfile.unlink() + return True + + +from pathlib import Path +print("can spawn?", can_spawn_tahoe(Path("running.process"))) diff --git a/docs/running.rst b/docs/running.rst index 406c8200b..2cff59928 100644 --- a/docs/running.rst +++ b/docs/running.rst @@ -124,6 +124,35 @@ Tahoe-LAFS. .. _magic wormhole: https://magic-wormhole.io/ +Multiple Instances +------------------ + +Running multiple instances against the same configuration directory isn't supported. +This will lead to undefined behavior and could corrupt the configuration state. + +We attempt to avoid this situation with a "pidfile"-style file in the config directory called ``running.process``. +There may be a parallel file called ``running.process.lock`` in existence. + +The ``.lock`` file exists to make sure only one process modifies ``running.process`` at once. +The lock file is managed by the `lockfile `_ library. +If you wish to make use of ``running.process`` for any reason you should also lock it and follow the semantics of lockfile. + +If ``running.process` exists it file contains the PID and the creation-time of the process. +When no such file exists, there is no other process running on this configuration. +If there is a ``running.process`` file, it may be a leftover file or it may indicate that another process is running against this config. +To tell the difference, determine if the PID in the file exists currently. +If it does, check the creation-time of the process versus the one in the file. +If these match, there is another process currently running. +Otherwise, the file is stale -- it should be removed before starting Tahoe-LAFS. + +Some example Python code to check the above situations: + +.. literalinclude:: check_running.py + + + + + A note about small grids ------------------------ From 930f4029f370313222b5c0872754f1db16434029 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 21 Sep 2022 20:07:46 -0600 Subject: [PATCH 27/44] properly write pid, create-time --- src/allmydata/util/pid.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/util/pid.py b/src/allmydata/util/pid.py index f965d72ab..1a833f285 100644 --- a/src/allmydata/util/pid.py +++ b/src/allmydata/util/pid.py @@ -94,9 +94,9 @@ def check_pid_process(pidfile): pidfile.remove() # write our PID + start-time to the pid-file - starttime = psutil.Process().create_time() + proc = psutil.Process() with pidfile.open("w") as f: - f.write("{} {}\n".format(pid, starttime).encode("utf8")) + f.write("{} {}\n".format(proc.pid, proc.create_time()).encode("utf8")) except Timeout: raise ProcessInTheWay( "Another process is still locking {}".format(pidfile.path) From 8474ecf83d46a25a473269f4f7907a5eb6e6e552 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 21 Sep 2022 20:15:07 -0600 Subject: [PATCH 28/44] typo --- src/allmydata/util/pid.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/util/pid.py b/src/allmydata/util/pid.py index 1a833f285..c13dc32f3 100644 --- a/src/allmydata/util/pid.py +++ b/src/allmydata/util/pid.py @@ -50,7 +50,7 @@ def parse_pidfile(pidfile): pidfile ) ) - return pid, startime + return pid, starttime def check_pid_process(pidfile): From fedea9696412d7397f58c11ea04a9148c55f8fd8 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 21 Sep 2022 20:26:14 -0600 Subject: [PATCH 29/44] less state --- src/allmydata/test/cli/test_run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/cli/test_run.py b/src/allmydata/test/cli/test_run.py index 551164d3c..e84f52096 100644 --- a/src/allmydata/test/cli/test_run.py +++ b/src/allmydata/test/cli/test_run.py @@ -168,7 +168,7 @@ class RunTests(SyncTestCase): config['basedir'] = basedir.path config.twistd_args = [] - from twisted.internet import reactor + reactor = MemoryReactor() runs = [] result_code = run(reactor, config, runApp=runs.append) From 8d8b0e6f01cdd8ab1f64593eda772a8b7db6c3d6 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 21 Sep 2022 20:40:25 -0600 Subject: [PATCH 30/44] cleanup --- src/allmydata/scripts/tahoe_run.py | 6 +----- src/allmydata/util/pid.py | 1 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/allmydata/scripts/tahoe_run.py b/src/allmydata/scripts/tahoe_run.py index 4d17492d4..e22e8c307 100644 --- a/src/allmydata/scripts/tahoe_run.py +++ b/src/allmydata/scripts/tahoe_run.py @@ -62,13 +62,9 @@ def get_pid_from_pidfile(pidfile): inaccessible, ``-1`` if PID file invalid. """ try: - with open(pidfile, "r") as f: - data = f.read().strip() + pid, _ = parse_pidfile(pidfile) except EnvironmentError: return None - - try: - pid, _ = parse_pidfile(pidfile) except InvalidPidFile: return -1 diff --git a/src/allmydata/util/pid.py b/src/allmydata/util/pid.py index c13dc32f3..f12c201d1 100644 --- a/src/allmydata/util/pid.py +++ b/src/allmydata/util/pid.py @@ -1,4 +1,3 @@ -import os import psutil # the docs are a little misleading, but this is either WindowsFileLock From 4f5a1ac37222e51974bcb0a28b5ec9e0e6c0e944 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 21 Sep 2022 23:36:23 -0600 Subject: [PATCH 31/44] naming? --- src/allmydata/test/test_runner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 5a8311649..962dffd1a 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -638,7 +638,7 @@ class PidFileLocking(SyncTestCase): pidfile = FilePath("foo") lockfile = _pidfile_to_lockpath(pidfile) - with open("code.py", "w") as f: + with open("other_lock.py", "w") as f: f.write( "\n".join([ "import filelock, time", @@ -648,7 +648,7 @@ class PidFileLocking(SyncTestCase): ]) ) proc = Popen( - [sys.executable, "code.py"], + [sys.executable, "other_lock.py"], stdout=PIPE, stderr=PIPE, start_new_session=True, From 8ebe331c358789f3af8dd9d64607ee63404077d7 Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 22 Sep 2022 00:11:20 -0600 Subject: [PATCH 32/44] maybe a newline helps --- src/allmydata/test/test_runner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 962dffd1a..3d8180c7a 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -643,7 +643,7 @@ class PidFileLocking(SyncTestCase): "\n".join([ "import filelock, time", "with filelock.FileLock('{}', timeout=1):".format(lockfile.path), - " print('.', flush=True)", + " print('.\n', flush=True)", " time.sleep(5)", ]) ) @@ -657,7 +657,7 @@ class PidFileLocking(SyncTestCase): # for sure (from the "." it prints) self.assertThat( proc.stdout.read(1), - Equals(b".") + Equals(b".\n") ) # we should not be able to acuire this corresponding lock as well From a182a2507987213b519bcb22c6f49eec0004830c Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 22 Sep 2022 21:43:20 -0600 Subject: [PATCH 33/44] backslashes --- src/allmydata/test/test_runner.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 3d8180c7a..e6b7b746f 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -641,9 +641,10 @@ class PidFileLocking(SyncTestCase): with open("other_lock.py", "w") as f: f.write( "\n".join([ - "import filelock, time", + "import filelock, time, sys", "with filelock.FileLock('{}', timeout=1):".format(lockfile.path), - " print('.\n', flush=True)", + " sys.stdout.write('.\\n')", + " sys.stdout.flush()", " time.sleep(5)", ]) ) @@ -656,7 +657,7 @@ class PidFileLocking(SyncTestCase): # make sure our subprocess has had time to acquire the lock # for sure (from the "." it prints) self.assertThat( - proc.stdout.read(1), + proc.stdout.read(2), Equals(b".\n") ) From 62b92585c62c44694e5db7a8769a772b2d712a07 Mon Sep 17 00:00:00 2001 From: meejah Date: Thu, 22 Sep 2022 23:57:19 -0600 Subject: [PATCH 34/44] simplify --- src/allmydata/test/test_runner.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index e6b7b746f..5431fbaa9 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -656,12 +656,9 @@ class PidFileLocking(SyncTestCase): ) # make sure our subprocess has had time to acquire the lock # for sure (from the "." it prints) - self.assertThat( - proc.stdout.read(2), - Equals(b".\n") - ) + proc.stdout.read(2), - # we should not be able to acuire this corresponding lock as well + # acquiring the same lock should fail; it is locked by the subprocess with self.assertRaises(ProcessInTheWay): check_pid_process(pidfile) proc.terminate() From 7fdeb8797e8164f1b0fd15ddda4108417545e00d Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 23 Sep 2022 00:26:39 -0600 Subject: [PATCH 35/44] hardcoding bad --- src/allmydata/test/test_runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 5431fbaa9..f414ed8b3 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -635,7 +635,7 @@ class PidFileLocking(SyncTestCase): """ # this can't just be "our" process because the locking library # allows the same process to acquire a lock multiple times. - pidfile = FilePath("foo") + pidfile = FilePath(self.mktemp()) lockfile = _pidfile_to_lockpath(pidfile) with open("other_lock.py", "w") as f: From f2cfd96b5e3af0fe82a7bf1ef770cad08d3969cd Mon Sep 17 00:00:00 2001 From: meejah Date: Fri, 23 Sep 2022 01:04:58 -0600 Subject: [PATCH 36/44] typo, longer timeout --- src/allmydata/test/test_runner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index f414ed8b3..b80891642 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -645,7 +645,7 @@ class PidFileLocking(SyncTestCase): "with filelock.FileLock('{}', timeout=1):".format(lockfile.path), " sys.stdout.write('.\\n')", " sys.stdout.flush()", - " time.sleep(5)", + " time.sleep(10)", ]) ) proc = Popen( @@ -656,7 +656,7 @@ class PidFileLocking(SyncTestCase): ) # make sure our subprocess has had time to acquire the lock # for sure (from the "." it prints) - proc.stdout.read(2), + proc.stdout.read(2) # acquiring the same lock should fail; it is locked by the subprocess with self.assertRaises(ProcessInTheWay): From 8991509f8c82642d75e3070ad7ae02bfe061977d Mon Sep 17 00:00:00 2001 From: meejah Date: Sun, 25 Sep 2022 00:16:40 -0600 Subject: [PATCH 37/44] blackslashes.... --- src/allmydata/test/test_runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index b80891642..f8211ec02 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -642,7 +642,7 @@ class PidFileLocking(SyncTestCase): f.write( "\n".join([ "import filelock, time, sys", - "with filelock.FileLock('{}', timeout=1):".format(lockfile.path), + "with filelock.FileLock(r'{}', timeout=1):".format(lockfile.path), " sys.stdout.write('.\\n')", " sys.stdout.flush()", " time.sleep(10)", From d42c00ae9293dd18c9f1efd22e86984c4725f222 Mon Sep 17 00:00:00 2001 From: meejah Date: Sun, 25 Sep 2022 00:46:30 -0600 Subject: [PATCH 38/44] do all checks with lock --- docs/check_running.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/docs/check_running.py b/docs/check_running.py index ecc55da34..55aae0015 100644 --- a/docs/check_running.py +++ b/docs/check_running.py @@ -21,22 +21,22 @@ def can_spawn_tahoe(pidfile): except FileNotFoundError: return True - # somewhat interesting: we have a pidfile - pid = int(pid) - create_time = float(create_time) + # somewhat interesting: we have a pidfile + pid = int(pid) + create_time = float(create_time) - try: - proc = psutil.Process(pid) - # most interesting case: there _is_ a process running at the - # recorded PID -- but did it just happen to get that PID, or - # is it the very same one that wrote the file? - if create_time == proc.create_time(): - # _not_ stale! another intance is still running against - # this configuration - return False + try: + proc = psutil.Process(pid) + # most interesting case: there _is_ a process running at the + # recorded PID -- but did it just happen to get that PID, or + # is it the very same one that wrote the file? + if create_time == proc.create_time(): + # _not_ stale! another intance is still running against + # this configuration + return False - except psutil.NoSuchProcess: - pass + except psutil.NoSuchProcess: + pass # the file is stale pidfile.unlink() From d16d233872df95b8e3876e3aa32e0fdb30cc9f98 Mon Sep 17 00:00:00 2001 From: meejah Date: Sun, 25 Sep 2022 00:47:58 -0600 Subject: [PATCH 39/44] wording --- docs/running.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/running.rst b/docs/running.rst index 2cff59928..b487f4ae3 100644 --- a/docs/running.rst +++ b/docs/running.rst @@ -128,7 +128,7 @@ Multiple Instances ------------------ Running multiple instances against the same configuration directory isn't supported. -This will lead to undefined behavior and could corrupt the configuration state. +This will lead to undefined behavior and could corrupt the configuration or state. We attempt to avoid this situation with a "pidfile"-style file in the config directory called ``running.process``. There may be a parallel file called ``running.process.lock`` in existence. From 4919b6d9066a10028e6548800a589929c3c094d9 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 28 Sep 2022 09:34:36 -0600 Subject: [PATCH 40/44] typo Co-authored-by: Jean-Paul Calderone --- docs/running.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/running.rst b/docs/running.rst index b487f4ae3..29df15a3c 100644 --- a/docs/running.rst +++ b/docs/running.rst @@ -137,7 +137,7 @@ The ``.lock`` file exists to make sure only one process modifies ``running.proce The lock file is managed by the `lockfile `_ library. If you wish to make use of ``running.process`` for any reason you should also lock it and follow the semantics of lockfile. -If ``running.process` exists it file contains the PID and the creation-time of the process. +If ``running.process`` exists then it contains the PID and the creation-time of the process. When no such file exists, there is no other process running on this configuration. If there is a ``running.process`` file, it may be a leftover file or it may indicate that another process is running against this config. To tell the difference, determine if the PID in the file exists currently. From 7aae2f78575541799002645e85a3aeab9f8706c2 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 28 Sep 2022 09:34:54 -0600 Subject: [PATCH 41/44] Clarify Co-authored-by: Jean-Paul Calderone --- docs/running.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/running.rst b/docs/running.rst index 29df15a3c..263448735 100644 --- a/docs/running.rst +++ b/docs/running.rst @@ -142,7 +142,7 @@ When no such file exists, there is no other process running on this configuratio If there is a ``running.process`` file, it may be a leftover file or it may indicate that another process is running against this config. To tell the difference, determine if the PID in the file exists currently. If it does, check the creation-time of the process versus the one in the file. -If these match, there is another process currently running. +If these match, there is another process currently running and using this config. Otherwise, the file is stale -- it should be removed before starting Tahoe-LAFS. Some example Python code to check the above situations: From a7398e13f7c82707738c3862cb085d7e2a055bb2 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 28 Sep 2022 09:35:17 -0600 Subject: [PATCH 42/44] Update docs/check_running.py Co-authored-by: Jean-Paul Calderone --- docs/check_running.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/check_running.py b/docs/check_running.py index 55aae0015..2705f1721 100644 --- a/docs/check_running.py +++ b/docs/check_running.py @@ -38,9 +38,9 @@ def can_spawn_tahoe(pidfile): except psutil.NoSuchProcess: pass - # the file is stale - pidfile.unlink() - return True + # the file is stale + pidfile.unlink() + return True from pathlib import Path From ca522a5293c8f2e38e9b8d2071fc3865907f4177 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 28 Sep 2022 10:07:44 -0600 Subject: [PATCH 43/44] sys.argv not inline --- src/allmydata/test/test_runner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index f8211ec02..00c87ce08 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -642,14 +642,14 @@ class PidFileLocking(SyncTestCase): f.write( "\n".join([ "import filelock, time, sys", - "with filelock.FileLock(r'{}', timeout=1):".format(lockfile.path), + "with filelock.FileLock(sys.argv[1], timeout=1):", " sys.stdout.write('.\\n')", " sys.stdout.flush()", " time.sleep(10)", ]) ) proc = Popen( - [sys.executable, "other_lock.py"], + [sys.executable, "other_lock.py", lockfile.path], stdout=PIPE, stderr=PIPE, start_new_session=True, From bef71978b6e7181598fc30f1b94d56b0b7e6a7c5 Mon Sep 17 00:00:00 2001 From: meejah Date: Wed, 28 Sep 2022 10:08:13 -0600 Subject: [PATCH 44/44] don't need start_new_session --- src/allmydata/test/test_runner.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 00c87ce08..74e3f803e 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -652,7 +652,6 @@ class PidFileLocking(SyncTestCase): [sys.executable, "other_lock.py", lockfile.path], stdout=PIPE, stderr=PIPE, - start_new_session=True, ) # make sure our subprocess has had time to acquire the lock # for sure (from the "." it prints)