Merge "Fix a race condition in edit monitor" into main (4cdd8ec8) · Commits · e / os / android_build

tools/edit_monitor/daemon_manager.py

+30 −12

Original line number	Diff line number	Diff line
		@@ -149,15 +149,15 @@ class DaemonManager:
		edit_event_pb2.EditEvent.KILLED_DUE_TO_EXCEEDED_MEMORY_USAGE
		)
		logging.error(
		"Daemon process is consuming too much memory, rebooting...")
		"Daemon process is consuming too much memory, rebooting..."
		)
		self.reboot()

		if self.max_cpu_usage >= cpu_threshold:
		self._send_error_event_to_clearcut(
		edit_event_pb2.EditEvent.KILLED_DUE_TO_EXCEEDED_CPU_USAGE
		)
		logging.error(
		"Daemon process is consuming too much cpu, killing...")
		logging.error("Daemon process is consuming too much cpu, killing...")
		self._terminate_process(self.daemon_process.pid)

		logging.info(
		@@ -179,7 +179,7 @@ class DaemonManager:
		self._wait_for_process_terminate(self.daemon_process.pid, 1)
		if self.daemon_process.is_alive():
		self._terminate_process(self.daemon_process.pid)
		self._remove_pidfile()
		self._remove_pidfile(self.pid)
		logging.info("Successfully stopped daemon manager.")
		except Exception as e:
		logging.exception("Failed to stop daemon manager with error %s", e)
		@@ -253,11 +253,15 @@ class DaemonManager:
		if ex_pid:
		logging.info("Found another instance with pid %d.", ex_pid)
		self._terminate_process(ex_pid)
		self._remove_pidfile()
		self._remove_pidfile(ex_pid)

		def _read_pid_from_pidfile(self):
		def _read_pid_from_pidfile(self) -> int \| None:
		try:
		with open(self.pid_file_path, "r") as f:
		return int(f.read().strip())
		except FileNotFoundError as e:
		logging.warning("pidfile %s does not exist.", self.pid_file_path)
		return None

		def _write_pid_to_pidfile(self):
		"""Creates a pidfile and writes the current pid to the file.
		@@ -333,7 +337,23 @@ class DaemonManager:
		)
		return True

		def _remove_pidfile(self):
		def _remove_pidfile(self, expected_pid: int):
		recorded_pid = self._read_pid_from_pidfile()

		if recorded_pid is None:
		logging.info("pid file %s already removed.", self.pid_file_path)
		return

		if recorded_pid != expected_pid:
		logging.warning(
		"pid file contains pid from a different process, expected pid: %d,"
		" actual pid: %d.",
		expected_pid,
		recorded_pid,
		)
		return

		logging.debug("removing pidfile written by process %s", expected_pid)
		try:
		os.remove(self.pid_file_path)
		except FileNotFoundError:
		@@ -378,9 +398,7 @@ class DaemonManager:
		uptime_end = float(f.readline().split()[0])

		return (
		(total_end_time - total_start_time)
		/ (uptime_end - uptime_start)
		* 100
		(total_end_time - total_start_time) / (uptime_end - uptime_start) * 100
		)

		def _get_total_cpu_time(self, pid: int) -> float:

tools/edit_monitor/edit_monitor_integration_test.py

+33 −5

Original line number	Diff line number	Diff line
		@@ -15,6 +15,7 @@
		"""Integration tests for Edit Monitor."""

		import glob
		from importlib import resources
		import logging
		import os
		import pathlib
		@@ -25,8 +26,6 @@ import sys
		import tempfile
		import time
		import unittest

		from importlib import resources
		from unittest import mock


		@@ -49,7 +48,8 @@ class EditMonitorIntegrationTest(unittest.TestCase):
		self.root_monitoring_path.mkdir()
		self.edit_monitor_binary_path = self._import_executable("edit_monitor")
		self.patch = mock.patch.dict(
		os.environ, {'ENABLE_ANDROID_EDIT_MONITOR': 'true'})
		os.environ, {"ENABLE_ANDROID_EDIT_MONITOR": "true"}
		)
		self.patch.start()

		def tearDown(self):
		@@ -83,7 +83,21 @@ class EditMonitorIntegrationTest(unittest.TestCase):

		self.assertEqual(self._get_logged_events_num(), 4)

		def _start_edit_monitor_process(self):
		def test_start_multiple_edit_monitor_only_one_started(self):
		p1 = self._start_edit_monitor_process(wait_for_observer_start=False)
		p2 = self._start_edit_monitor_process(wait_for_observer_start=False)
		p3 = self._start_edit_monitor_process(wait_for_observer_start=False)

		live_processes = self._get_live_processes([p1, p2, p3])

		# Cleanup all live processes.
		for p in live_processes:
		os.kill(p.pid, signal.SIGINT)
		p.communicate()

		self.assertEqual(len(live_processes), 1)

		def _start_edit_monitor_process(self, wait_for_observer_start=True):
		command = f"""
		export TMPDIR="{self.working_dir.name}"
		{self.edit_monitor_binary_path} --path={self.root_monitoring_path} --dry_run"""
		@@ -94,7 +108,9 @@ class EditMonitorIntegrationTest(unittest.TestCase):
		start_new_session=True,
		executable="/bin/bash",
		)
		if wait_for_observer_start:
		self._wait_for_observer_start(time_out=5)

		return p

		def _wait_for_observer_start(self, time_out):
		@@ -125,6 +141,18 @@ class EditMonitorIntegrationTest(unittest.TestCase):

		return 0

		def _get_live_processes(self, processes):
		live_processes = []
		for p in processes:
		try:
		p.wait(timeout=5)
		except subprocess.TimeoutExpired as e:
		live_processes.append(p)
		logging.info("process: %d still alive.", p.pid)
		else:
		logging.info("process: %d stopped.", p.pid)
		return live_processes

		def _import_executable(self, executable_name: str) -> pathlib.Path:
		binary_dir = pathlib.Path(self.working_dir.name).joinpath("binary")
		binary_dir.mkdir()

tools/edit_monitor/main.py

+2 −1

Original line number	Diff line number	Diff line
		@@ -72,7 +72,8 @@ def configure_logging(verbose=False):
		root_logging_dir = tempfile.mkdtemp(prefix='edit_monitor_')
		_, log_path = tempfile.mkstemp(dir=root_logging_dir, suffix='.log')

		log_fmt = '%(asctime)s %(filename)s:%(lineno)s:%(levelname)s: %(message)s'

		log_fmt = '%(asctime)s.%(msecs)03d %(filename)s:%(lineno)s:%(levelname)s: %(message)s'
		date_fmt = '%Y-%m-%d %H:%M:%S'
		log_level = logging.DEBUG if verbose else logging.INFO