Optimize Android emulator initialization by batching reboots

src/clusterfuzz/_internal/platforms/android/adb.py

@@ -880,7 +880,7 @@ def write_command_line_file(command_line, app_path):
   write_data_to_file(command_line_file_contents, command_line_path)
 
 
-def write_data_to_file(contents, file_path):
+def write_data_to_file(contents, file_path, wait_for_reboot=True):
   """Writes content to file."""
   # If this is a file in /system, we need to remount /system as read-write and
   # after file is written, revert it back to read-only.
@@ -895,6 +895,6 @@ def write_data_to_file(contents, file_path):
   # Make command line file is readable.
   run_shell_command('chmod 0644 %s' % file_path, root=True)
 
-  if is_system_file:
+  if is_system_file and wait_for_reboot:
     reboot()
     wait_until_fully_booted()

src/clusterfuzz/_internal/platforms/android/device.py

@@ -183,9 +183,13 @@ def configure_device_settings():
   adb.write_data_to_file(local_properties_file_contents, LOCAL_PROP_PATH)
 
 
-def configure_system_build_properties():
+def configure_system_build_properties() -> bool:
   """Modifies system build properties in /system/build.prop for better boot
-  speed and power use."""
+  speed and power use.
+
+  Returns:
+    True if the device was rebooted during configuration, False otherwise.
+  """
   adb.run_as_root()
 
   # Check md5 checksum of build.prop to see if already updated,
@@ -195,17 +199,17 @@ def configure_system_build_properties():
   current_md5 = adb.get_file_checksum(BUILD_PROP_PATH)
   if current_md5 is None:
     logs.error('Unable to find %s on device.' % BUILD_PROP_PATH)
-    return
+    return False
   if old_md5 == current_md5:
-    return
+    return False
 
   # Pull to tmp file.
   bot_tmp_directory = environment.get_value('BOT_TMPDIR')
   old_build_prop_path = os.path.join(bot_tmp_directory, 'old.prop')
   adb.run_command(['pull', BUILD_PROP_PATH, old_build_prop_path])
   if not os.path.exists(old_build_prop_path):
     logs.error('Unable to fetch %s from device.' % BUILD_PROP_PATH)
-    return
+    return False
 
   # Write new build.prop.
   new_build_prop_path = os.path.join(bot_tmp_directory, 'new.prop')
@@ -255,6 +259,8 @@ def configure_system_build_properties():
   current_md5 = adb.get_file_checksum(BUILD_PROP_PATH)
   persistent_cache.set_value(constants.BUILD_PROP_MD5_KEY, current_md5)
 
+  return True
+
 
 def get_debug_props_and_values():
   """Return debug property names and values based on |ENABLE_DEBUG_CHECKS|
@@ -306,16 +312,17 @@ def initialize_device():
   adb.setup_adb()
 
   # General device configuration settings.
-  configure_system_build_properties()
+  needs_full_reboot = configure_system_build_properties()
   configure_device_settings()
   add_test_accounts_if_needed()
 
   # Setup AddressSanitizer if needed.
-  sanitizer.setup_asan_if_needed()
+  asan_reboot_done = sanitizer.setup_asan_if_needed()
 
   # Reboot device as above steps would need it and also it brings device in a
   # good state.
-  reboot()
+  if needs_full_reboot or not asan_reboot_done:
+    reboot()
 
   # Make sure we are running as root after restart.
   adb.run_as_root()

src/clusterfuzz/_internal/platforms/android/sanitizer.py

@@ -69,25 +69,31 @@ def set_options(sanitizer_tool_name, sanitizer_options):
   if not sanitizer_options_file_path:
     return
 
-  adb.write_data_to_file(sanitizer_options, sanitizer_options_file_path)
+  # Skip reboot as the app will pick up the options file on restart.
+  adb.write_data_to_file(
+      sanitizer_options, sanitizer_options_file_path, wait_for_reboot=False)
 
 
-def setup_asan_if_needed():
-  """Set up asan on device."""
+def setup_asan_if_needed() -> bool:
+  """Set up asan on device.
+
+  Returns:
+    True if the device was rebooted or restarted during setup, False otherwise.
+  """
   if not environment.get_value('ASAN_DEVICE_SETUP'):
     # Only do this step if explicitly enabled in the job type. This cannot be
     # determined from libraries in application directory since they can go
     # missing in a bad build, so we want to catch that.
-    return
+    return False
 
   if settings.get_sanitizer_tool_name():
     # If this is a sanitizer build, no need to setup ASAN (incompatible).
-    return
+    return False
 
   app_directory = environment.get_value('APP_DIR')
   if not app_directory:
     # No app directory -> No ASAN runtime library. No work to do, bail out.
-    return
+    return False
 
   # Initialize variables.
   android_directory = environment.get_platform_resources_directory()
@@ -108,7 +114,7 @@ def setup_asan_if_needed():
   result = process.run_and_wait()
   if result.return_code:
     logs.error('Failed to setup ASan on device.', output=result.output)
-    return
+    return False
 
   logs.info(
       'ASan device setup script successfully finished, waiting for boot.',
@@ -117,3 +123,5 @@ def setup_asan_if_needed():
   # Wait until fully booted as otherwise shell restart followed by a quick
   # reboot can trigger data corruption in /data/data.
   adb.wait_until_fully_booted()
+
+  return True

src/clusterfuzz/_internal/tests/core/platforms/android/device_test.py

@@ -13,6 +13,9 @@
 # limitations under the License.
 """Tests for device functions."""
 
+import unittest
+from unittest import mock
+
 from clusterfuzz._internal.platforms.android import device
 from clusterfuzz._internal.tests.test_libs import android_helpers
 
@@ -23,3 +26,79 @@ class InitializeEnvironmentTest(android_helpers.AndroidTest):
   def test(self):
     """Ensure that initialize_environment throws no exceptions."""
     device.initialize_environment()
+
+
+class InitializeDeviceRebootLogicTest(unittest.TestCase):
+  """Tests the reboot batching logic in initialize_device."""
+
+  def setUp(self):
+    # Mock environment to bypass the engine fuzzer check
+    mock.patch(
+        'clusterfuzz._internal.system.environment.is_engine_fuzzer_job',
+        return_value=False).start()
+
+    # Mock all the setup steps so we don't actually run ADB commands
+    self.mock_setup_adb = mock.patch(
+        'clusterfuzz._internal.platforms.android.adb.setup_adb').start()
+    self.mock_run_as_root = mock.patch(
+        'clusterfuzz._internal.platforms.android.adb.run_as_root').start()
+    self.mock_config_props = mock.patch(
+        'clusterfuzz._internal.platforms.android.device.configure_system_build_properties'
+    ).start()
+    self.mock_config_settings = mock.patch(
+        'clusterfuzz._internal.platforms.android.device.configure_device_settings'
+    ).start()
+    self.mock_add_accounts = mock.patch(
+        'clusterfuzz._internal.platforms.android.device.add_test_accounts_if_needed'
+    ).start()
+    self.mock_setup_asan = mock.patch(
+        'clusterfuzz._internal.platforms.android.sanitizer.setup_asan_if_needed'
+    ).start()
+
+    # Mock the reboot function we are trying to track
+    self.mock_reboot = mock.patch(
+        'clusterfuzz._internal.platforms.android.device.reboot').start()
+
+    # Mock the post-reboot steps
+    mock.patch('clusterfuzz._internal.platforms.android.wifi.configure').start()
+    mock.patch(
+        'clusterfuzz._internal.platforms.android.device.setup_host_and_device_forwarder_if_needed'
+    ).start()
+    mock.patch(
+        'clusterfuzz._internal.platforms.android.settings.change_se_linux_to_permissive_mode'
+    ).start()
+    mock.patch(
+        'clusterfuzz._internal.platforms.android.app.wait_until_optimization_complete'
+    ).start()
+    mock.patch('clusterfuzz._internal.platforms.android.ui.clear_notifications'
+              ).start()
+    mock.patch(
+        'clusterfuzz._internal.platforms.android.ui.unlock_screen').start()
+
+  def tearDown(self):
+    mock.patch.stopall()
+
+  def test_reboot_if_props_changed(self):
+    """Test that it reboots if build.prop changed, even if ASan didn't run."""
+    self.mock_config_props.return_value = True
+    self.mock_setup_asan.return_value = False
+
+    device.initialize_device()
+    self.mock_reboot.assert_called_once()
+
+  def test_no_reboot_if_asan_ran(self):
+    """Test that the final reboot is skipped if ASan setup performed a shell restart."""
+    # If build.prop didn't change, the ASan restart handles the clean state.
+    self.mock_config_props.return_value = False
+    self.mock_setup_asan.return_value = True
+
+    device.initialize_device()
+    self.mock_reboot.assert_not_called()
+
+  def test_reboot_if_clean_slate_needed(self):
+    """Test that it still reboots to ensure a clean state if no other steps restarted it."""
+    self.mock_config_props.return_value = False
+    self.mock_setup_asan.return_value = False
+
+    device.initialize_device()
+    self.mock_reboot.assert_called_once()