diff -Nru landscape-client-19.12/debian/changelog landscape-client-19.12/debian/changelog --- landscape-client-19.12/debian/changelog 2020-01-25 04:32:59.000000000 +0000 +++ landscape-client-19.12/debian/changelog 2020-04-14 15:07:41.000000000 +0000 @@ -1,3 +1,10 @@ +landscape-client (19.12-0ubuntu4) focal; urgency=medium + + * d/p/0002-lp1870087-stale-locks.patch: + Clean stale twisted lock files (LP: #1870087) + + -- Simon Poirier Tue, 14 Apr 2020 11:07:41 -0400 + landscape-client (19.12-0ubuntu3) focal; urgency=medium * No-change rebuild to build with python3.8. diff -Nru landscape-client-19.12/debian/patches/0002-lp1870087-stale-locks.patch landscape-client-19.12/debian/patches/0002-lp1870087-stale-locks.patch --- landscape-client-19.12/debian/patches/0002-lp1870087-stale-locks.patch 1970-01-01 00:00:00.000000000 +0000 +++ landscape-client-19.12/debian/patches/0002-lp1870087-stale-locks.patch 2020-04-14 15:07:41.000000000 +0000 @@ -0,0 +1,210 @@ +Description: Clean up stale lock files + Rework lockfile to look for process names. +Author: Simon Poirier +Origin: upstream, https://github.com/CanonicalLtd/landscape-client/commit/2450eacd331097431122b9861613b8a03e5f74d9 +Bug-Ubuntu: https://bugs.launchpad.net/bugs/1870087 + +Index: landscape-client-19.12/landscape/client/lockfile.py +=================================================================== +--- /dev/null ++++ landscape-client-19.12/landscape/client/lockfile.py +@@ -0,0 +1,52 @@ ++import os ++ ++from twisted.python import lockfile ++ ++ ++def patch_lockfile(): ++ if lockfile.FilesystemLock is PatchedFilesystemLock: ++ return ++ lockfile.FilesystemLock = PatchedFilesystemLock ++ ++ ++class PatchedFilesystemLock(lockfile.FilesystemLock): ++ """ ++ Patched Twisted's FilesystemLock.lock to handle PermissionError ++ when trying to lock. ++ """ ++ ++ def lock(self): ++ # XXX Twisted assumes PIDs don't get reused, which is incorrect. ++ # As such, we pre-check that any existing lock file isn't ++ # associated to a live process, and that any associated ++ # process is from landscape. Otherwise, clean up the lock file, ++ # considering it to be locked to a recycled PID. ++ # ++ # Although looking for the process name may seem fragile, it's the ++ # most acurate info we have since: ++ # * some process run as root, so the UID is not a reference ++ # * process may not be spawned by systemd, so cgroups are not reliable ++ # * python executable is not a reference ++ clean = True ++ try: ++ pid = os.readlink(self.name) ++ ps_name = get_process_name(int(pid)) ++ if not ps_name.startswith("landscape"): ++ os.remove(self.name) ++ clean = False ++ except Exception: ++ # We can't figure the lock state, let FilesystemLock figure it ++ # out normally. ++ pass ++ ++ result = super(PatchedFilesystemLock, self).lock() ++ self.clean = self.clean and clean ++ return result ++ ++ ++def get_process_name(pid): ++ """Return a process name from a pid.""" ++ stat_path = "/proc/{}/stat".format(pid) ++ with open(stat_path) as stat_file: ++ stat = stat_file.read() ++ return stat.partition("(")[2].rpartition(")")[0] +Index: landscape-client-19.12/landscape/client/reactor.py +=================================================================== +--- landscape-client-19.12.orig/landscape/client/reactor.py ++++ landscape-client-19.12/landscape/client/reactor.py +@@ -2,6 +2,9 @@ + Extend the regular Twisted reactor with event-handling features. + """ + from landscape.lib.reactor import EventHandlingReactor ++from landscape.client.lockfile import patch_lockfile ++ ++patch_lockfile() + + + class LandscapeReactor(EventHandlingReactor): +Index: landscape-client-19.12/landscape/client/tests/test_amp.py +=================================================================== +--- landscape-client-19.12.orig/landscape/client/tests/test_amp.py ++++ landscape-client-19.12/landscape/client/tests/test_amp.py +@@ -1,9 +1,17 @@ +-from twisted.internet.error import ConnectError ++import os ++import errno ++import subprocess ++import textwrap ++ ++import mock ++ ++from twisted.internet.error import ConnectError, CannotListenError + from twisted.internet.task import Clock + + from landscape.client.tests.helpers import LandscapeTest + from landscape.client.deployment import Configuration + from landscape.client.amp import ComponentPublisher, ComponentConnector, remote ++from landscape.client.reactor import LandscapeReactor + from landscape.lib.amp import MethodCallError + from landscape.lib.testing import FakeReactor + +@@ -159,3 +167,83 @@ class ComponentConnectorTest(LandscapeTe + effectively a no-op. + """ + self.connector.disconnect() ++ ++ @mock.patch("twisted.python.lockfile.kill") ++ def test_stale_locks_with_dead_pid(self, mock_kill): ++ """Publisher starts with stale lock.""" ++ mock_kill.side_effect = [ ++ OSError(errno.ESRCH, "No such process")] ++ sock_path = os.path.join(self.config.sockets_path, u"test.sock") ++ lock_path = u"{}.lock".format(sock_path) ++ # fake a PID which does not exist ++ os.symlink("-1", lock_path) ++ ++ component = TestComponent() ++ # Test the actual Unix reactor implementation. Fakes won't do. ++ reactor = LandscapeReactor() ++ publisher = ComponentPublisher(component, reactor, self.config) ++ ++ # Shouldn't raise the exception. ++ publisher.start() ++ ++ # ensure stale lock was replaced ++ self.assertNotEqual("-1", os.readlink(lock_path)) ++ mock_kill.assert_called_with(-1, 0) ++ ++ publisher.stop() ++ reactor._cleanup() ++ ++ @mock.patch("twisted.python.lockfile.kill") ++ def test_stale_locks_recycled_pid(self, mock_kill): ++ """Publisher starts with stale lock pointing to recycled process.""" ++ mock_kill.side_effect = [ ++ OSError(errno.EPERM, "Operation not permitted")] ++ sock_path = os.path.join(self.config.sockets_path, u"test.sock") ++ lock_path = u"{}.lock".format(sock_path) ++ # fake a PID recycled by a known process which isn't landscape (init) ++ os.symlink("1", lock_path) ++ ++ component = TestComponent() ++ # Test the actual Unix reactor implementation. Fakes won't do. ++ reactor = LandscapeReactor() ++ publisher = ComponentPublisher(component, reactor, self.config) ++ ++ # Shouldn't raise the exception. ++ publisher.start() ++ ++ # ensure stale lock was replaced ++ self.assertNotEqual("1", os.readlink(lock_path)) ++ mock_kill.assert_not_called() ++ self.assertFalse(publisher._port.lockFile.clean) ++ ++ publisher.stop() ++ reactor._cleanup() ++ ++ @mock.patch("twisted.python.lockfile.kill") ++ def test_with_valid_lock(self, mock_kill): ++ """Publisher raises lock error if a valid lock is held.""" ++ sock_path = os.path.join(self.config.sockets_path, u"test.sock") ++ lock_path = u"{}.lock".format(sock_path) ++ # fake a landscape process ++ app = self.makeFile(textwrap.dedent("""\ ++ #!/usr/bin/python3 ++ import time ++ time.sleep(10) ++ """), basename="landscape-manager") ++ os.chmod(app, 0o755) ++ call = subprocess.Popen([app]) ++ self.addCleanup(call.terminate) ++ os.symlink(str(call.pid), lock_path) ++ ++ component = TestComponent() ++ # Test the actual Unix reactor implementation. Fakes won't do. ++ reactor = LandscapeReactor() ++ publisher = ComponentPublisher(component, reactor, self.config) ++ ++ with self.assertRaises(CannotListenError): ++ publisher.start() ++ ++ # ensure lock was not replaced ++ self.assertEqual(str(call.pid), os.readlink(lock_path)) ++ mock_kill.assert_called_with(call.pid, 0) ++ reactor._cleanup() +Index: landscape-client-19.12/landscape/client/tests/test_lockfile.py +=================================================================== +--- /dev/null ++++ landscape-client-19.12/landscape/client/tests/test_lockfile.py +@@ -0,0 +1,21 @@ ++import os ++import subprocess ++import textwrap ++ ++from landscape.client import lockfile ++from landscape.client.tests.helpers import LandscapeTest ++ ++ ++class LockFileTest(LandscapeTest): ++ ++ def test_read_process_name(self): ++ app = self.makeFile(textwrap.dedent("""\ ++ #!/usr/bin/python3 ++ import time ++ time.sleep(10) ++ """), basename="my_fancy_app") ++ os.chmod(app, 0o755) ++ call = subprocess.Popen([app]) ++ self.addCleanup(call.terminate) ++ proc_name = lockfile.get_process_name(call.pid) ++ self.assertEqual("my_fancy_app", proc_name) diff -Nru landscape-client-19.12/debian/patches/series landscape-client-19.12/debian/patches/series --- landscape-client-19.12/debian/patches/series 2019-12-07 14:51:50.000000000 +0000 +++ landscape-client-19.12/debian/patches/series 2020-04-14 15:07:41.000000000 +0000 @@ -1 +1,2 @@ 0001-Handle-EINVAL-error-of-SIOCETHTOOL-ioctl.patch +0002-lp1870087-stale-locks.patch