Ensure a single running instance of edit monitor

This cl ensures at most 1 instance of edit monitor running from the
same binary by killing any existing instance before starting.

Specifically, When an edit monitor process starts, it will write a pidfile containing its pid and if such pidfile already exists (which means there's another instance there), it will read the pid contained in that pidfile and kill the corresponding process first.

Test: atst daemon_manager_test
bug: 365617369
Change-Id: I76954344df649aa79a6ef07ce55a62985decdb53

tools/edit_monitor/daemon_manager.py

@@ -55,6 +55,7 @@ class DaemonManager:
   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 @@ class DaemonManager:
     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.
 

tools/edit_monitor/daemon_manager_test.py

@@ -33,7 +33,7 @@ TEST_PID_FILE_PATH = (
 )
 
 
-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 @@ class DaemonManagerTest(unittest.TestCase):
     tempfile.tempdir = self.original_tempdir
     super().tearDown()
 
-  def test_start_success(self):
+  def test_start_success_with_no_existing_instance(self):
-    damone_output_file = tempfile.NamedTemporaryFile(
+    self.assert_run_simple_daemon_success()
-        dir=self.working_dir.name, delete=False
+
+  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(
+    pid_file_path_dir.mkdir(parents=True, exist_ok=True)
-        TEST_BINARY_FILE,
+    with open(pid_file_path_dir.joinpath(TEST_PID_FILE_PATH), 'w') as f:
-        daemon_target=example_daemon,
+      f.write(str(p.pid))
-        daemon_args=(damone_output_file.name,),
+
+    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.
+    # Verify no daemon process is started.
-    expected_pid_file_path = pathlib.Path(self.working_dir.name).joinpath(
+    self.assertIsNone(dm.daemon_process)
-        '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')
 
   def test_start_failed_to_write_pidfile(self):
     pid_file_path_dir = pathlib.Path(self.working_dir.name).joinpath(
@@ -151,6 +184,29 @@ class DaemonManagerTest(unittest.TestCase):
     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: