diff --git a/src/clusterfuzz/_internal/platforms/android/adb.py b/src/clusterfuzz/_internal/platforms/android/adb.py index 47c59b31c72..1f5eefcd861 100755 --- a/src/clusterfuzz/_internal/platforms/android/adb.py +++ b/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() diff --git a/src/clusterfuzz/_internal/platforms/android/device.py b/src/clusterfuzz/_internal/platforms/android/device.py index 0c8d81ca686..5a4461a84a5 100755 --- a/src/clusterfuzz/_internal/platforms/android/device.py +++ b/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,9 +199,9 @@ 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') @@ -205,7 +209,7 @@ def configure_system_build_properties(): 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() diff --git a/src/clusterfuzz/_internal/platforms/android/sanitizer.py b/src/clusterfuzz/_internal/platforms/android/sanitizer.py index 832f6a4a577..de51bd4207b 100644 --- a/src/clusterfuzz/_internal/platforms/android/sanitizer.py +++ b/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 diff --git a/src/clusterfuzz/_internal/tests/core/platforms/android/device_test.py b/src/clusterfuzz/_internal/tests/core/platforms/android/device_test.py index e10051fb7ef..7a5f792f545 100644 --- a/src/clusterfuzz/_internal/tests/core/platforms/android/device_test.py +++ b/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()