Merge "Ensure a single running instance of edit monitor" into main
diff --git a/tools/edit_monitor/daemon_manager.py b/tools/edit_monitor/daemon_manager.py
index c09c321..8ec2588 100644
--- a/tools/edit_monitor/daemon_manager.py
+++ b/tools/edit_monitor/daemon_manager.py
@@ -55,6 +55,7 @@
def start(self):
"""Writes the pidfile and starts the daemon proces."""
try:
+ self._stop_any_existing_instance()
self._write_pid_to_pidfile()
self._start_daemon_process()
except Exception as e:
@@ -71,6 +72,22 @@
except Exception as e:
logging.exception("Failed to stop daemon manager with error %s", e)
+ def _stop_any_existing_instance(self):
+ if not self.pid_file_path.exists():
+ logging.debug("No existing instances.")
+ return
+
+ ex_pid = self._read_pid_from_pidfile()
+
+ if ex_pid:
+ logging.info("Found another instance with pid %d.", ex_pid)
+ self._terminate_process(ex_pid)
+ self._remove_pidfile()
+
+ def _read_pid_from_pidfile(self):
+ with open(self.pid_file_path, "r") as f:
+ return int(f.read().strip())
+
def _write_pid_to_pidfile(self):
"""Creates a pidfile and writes the current pid to the file.
diff --git a/tools/edit_monitor/daemon_manager_test.py b/tools/edit_monitor/daemon_manager_test.py
index 5be4bee..214b038 100644
--- a/tools/edit_monitor/daemon_manager_test.py
+++ b/tools/edit_monitor/daemon_manager_test.py
@@ -33,7 +33,7 @@
)
-def example_daemon(output_file):
+def simple_daemon(output_file):
with open(output_file, 'w') as f:
f.write('running daemon target')
@@ -69,29 +69,62 @@
tempfile.tempdir = self.original_tempdir
super().tearDown()
- def test_start_success(self):
- damone_output_file = tempfile.NamedTemporaryFile(
- dir=self.working_dir.name, delete=False
+ def test_start_success_with_no_existing_instance(self):
+ self.assert_run_simple_daemon_success()
+
+ def test_start_success_with_existing_instance_running(self):
+ # Create a long running subprocess
+ p = multiprocessing.Process(target=long_running_daemon)
+ p.start()
+
+ # Create a pidfile with the subprocess pid
+ pid_file_path_dir = pathlib.Path(self.working_dir.name).joinpath(
+ 'edit_monitor'
)
- dm = daemon_manager.DaemonManager(
- TEST_BINARY_FILE,
- daemon_target=example_daemon,
- daemon_args=(damone_output_file.name,),
+ pid_file_path_dir.mkdir(parents=True, exist_ok=True)
+ with open(pid_file_path_dir.joinpath(TEST_PID_FILE_PATH), 'w') as f:
+ f.write(str(p.pid))
+
+ self.assert_run_simple_daemon_success()
+ p.terminate()
+
+ def test_start_success_with_existing_instance_already_dead(self):
+ # Create a pidfile with pid that does not exist.
+ pid_file_path_dir = pathlib.Path(self.working_dir.name).joinpath(
+ 'edit_monitor'
)
+ pid_file_path_dir.mkdir(parents=True, exist_ok=True)
+ with open(pid_file_path_dir.joinpath(TEST_PID_FILE_PATH), 'w') as f:
+ f.write('123456')
+
+ self.assert_run_simple_daemon_success()
+
+ def test_start_success_with_existing_instance_from_different_binary(self):
+ # First start an instance based on "some_binary_path"
+ existing_dm = daemon_manager.DaemonManager(
+ "some_binary_path",
+ daemon_target=long_running_daemon,
+ )
+ existing_dm.start()
+
+ self.assert_run_simple_daemon_success()
+ existing_dm.stop()
+
+ @mock.patch('os.kill')
+ def test_start_failed_to_kill_existing_instance(self, mock_kill):
+ mock_kill.side_effect = OSError('Unknown OSError')
+ pid_file_path_dir = pathlib.Path(self.working_dir.name).joinpath(
+ 'edit_monitor'
+ )
+ pid_file_path_dir.mkdir(parents=True, exist_ok=True)
+ with open(pid_file_path_dir.joinpath(TEST_PID_FILE_PATH), 'w') as f:
+ f.write('123456')
+
+ dm = daemon_manager.DaemonManager(TEST_BINARY_FILE)
dm.start()
- dm.daemon_process.join()
- # Verifies the expected pid file is created.
- expected_pid_file_path = pathlib.Path(self.working_dir.name).joinpath(
- 'edit_monitor', TEST_PID_FILE_PATH
- )
- self.assertEqual(dm.pid_file_path, expected_pid_file_path)
- self.assertTrue(expected_pid_file_path.exists())
-
- # Verifies the daemon process is executed successfully.
- with open(damone_output_file.name, 'r') as f:
- contents = f.read()
- self.assertEqual(contents, 'running daemon target')
+ # Verify no daemon process is started.
+ self.assertIsNone(dm.daemon_process)
def test_start_failed_to_write_pidfile(self):
pid_file_path_dir = pathlib.Path(self.working_dir.name).joinpath(
@@ -151,6 +184,29 @@
self.assert_no_subprocess_running()
self.assertTrue(dm.pid_file_path.exists())
+ def assert_run_simple_daemon_success(self):
+ damone_output_file = tempfile.NamedTemporaryFile(
+ dir=self.working_dir.name, delete=False
+ )
+ dm = daemon_manager.DaemonManager(
+ TEST_BINARY_FILE,
+ daemon_target=simple_daemon,
+ daemon_args=(damone_output_file.name,),
+ )
+ dm.start()
+ dm.daemon_process.join()
+
+ # Verifies the expected pid file is created.
+ expected_pid_file_path = pathlib.Path(self.working_dir.name).joinpath(
+ 'edit_monitor', TEST_PID_FILE_PATH
+ )
+ self.assertTrue(expected_pid_file_path.exists())
+
+ # Verify the daemon process is executed successfully.
+ with open(damone_output_file.name, 'r') as f:
+ contents = f.read()
+ self.assertEqual(contents, 'running daemon target')
+
def assert_no_subprocess_running(self):
child_pids = self._get_child_processes(os.getpid())
for child_pid in child_pids: