From 4a9881ff5b325f7163cbcededcd243129ad69864 Mon Sep 17 00:00:00 2001 From: firestar5683 <168790843+firestar5683@users.noreply.github.com> Date: Sat, 24 Oct 2026 12:25:09 -0500 Subject: [PATCH] Prevent synchronized crash-session uploads during driving --- openpilot/system/sentry.py | 10 ++--- openpilot/system/tests/test_sentry.py | 42 ++++++++++++++++++- prebuilt.json | 2 +- tools/ci/run_host_tests.py | 5 ++- tools/ci/tests/test_feature_test_discovery.py | 1 + 5 files changed, 49 insertions(+), 11 deletions(-) diff --git a/openpilot/system/sentry.py b/openpilot/system/sentry.py index 121c678b1f..bfc3a37781 100644 --- a/openpilot/system/sentry.py +++ b/openpilot/system/sentry.py @@ -58,7 +58,7 @@ def init(project: SentryProject = SentryProject.SELFDRIVE) -> bool: sdk.init(dsn=project.value, default_integrations=False, auto_enabling_integrations=False, integrations=[threading_module.ThreadingIntegration(propagate_scope=True)], transport=BoundedHttpTransport, transport_queue_size=TRANSPORT_QUEUE_SIZE, - shutdown_timeout=EXIT_FLUSH_TIMEOUT, enable_backpressure_handling=False, + shutdown_timeout=EXIT_FLUSH_TIMEOUT, enable_backpressure_handling=False, auto_session_tracking=False, max_value_length=8192, traces_sample_rate=0.0, release=os.getenv("GIT_COMMIT", "unknown"), environment="Release") _initialized_pid = os.getpid() @@ -86,11 +86,9 @@ def init(project: SentryProject = SentryProject.SELFDRIVE) -> bool: sdk.set_tag(key, str(value) if value is not None else "unknown") except Exception: cloudlog.warning("Crash reporting metadata unavailable") - if fresh: - try: - sdk.start_session() - except Exception: - cloudlog.warning("Crash reporting session unavailable") + # Forked daemons must not close and upload an inherited manager session. + # Its deferred 60-second flush synchronizes background TLS work across them. + # Explicit exceptions retain the bounded transport and threading integration. return True diff --git a/openpilot/system/tests/test_sentry.py b/openpilot/system/tests/test_sentry.py index af6eddc596..6544692bcc 100644 --- a/openpilot/system/tests/test_sentry.py +++ b/openpilot/system/tests/test_sentry.py @@ -98,7 +98,6 @@ def test_child_import_and_main_failure_after_real_fork(reporting, monkeypatch, d writer.send((sentry._initialized_pid, event_list(reporting))) monkeypatch.setattr(reporting.sdk, 'capture_exception', capture) - monkeypatch.setattr(process.messaging, 'reset_context', Mock()) monkeypatch.setattr(process, 'setproctitle', Mock()) monkeypatch.setattr(sentry, 'flush', Mock()) @@ -179,7 +178,9 @@ def test_native_apport_keeps_local_copy_and_queues_header_stack_qlog(reporting, monkeypatch.setattr(sentry, 'sdk', None) tombstoned.report_tombstone_apport(str(source)) assert not source.exists() - copies = list((reporting.tmp_path / 'crash').iterdir()) + crash_dir = reporting.tmp_path / 'crash' + assert (crash_dir / '.lock').is_file() + copies = [path for path in crash_dir.iterdir() if path.name != '.lock'] assert len(copies) == 1 and copies[0].read_text() == payload if not report_available: assert event_list(reporting) == [] @@ -279,3 +280,40 @@ def test_metadata_or_attachment_error_does_not_disable_capture(reporting, monkey sentry.capture_exception(ValueError('metadata and attachment unavailable')) event, = event_list(reporting) assert event['exception']['values'][0]['type'] == 'ValueError' + + +def test_forked_daemon_has_no_deferred_session_upload(reporting, monkeypatch): + ctx = multiprocessing.get_context('fork') + reader, writer = ctx.Pipe(duplex=False) + + def child(): + try: + assert sentry.init() + client = reporting.sdk.get_client() + pending = list(client.session_flusher.pending_sessions) + try: + raise RuntimeError('child crash still reported without session telemetry') + except RuntimeError: + reporting.sdk.capture_exception() + writer.send((pending, event_list(reporting), client.options['auto_session_tracking'])) + finally: + writer.close() + + process = ctx.Process(target=child) + process.start() + writer.close() + try: + assert reader.poll(5), 'child did not report its SDK state' + pending, events, automatic = reader.recv() + process.join(5) + assert not process.is_alive() and process.exitcode == 0 + assert pending == [], 'inherited manager session schedules a 60-second upload' + assert automatic is False + event, = events + assert event['exception']['values'][0]['value'] == 'child crash still reported without session telemetry' + assert event['user']['id'] == 'test-device' + finally: + if process.is_alive(): + process.kill() + process.join(5) + reader.close() diff --git a/prebuilt.json b/prebuilt.json index f6f4dcbd10..adbe6fe411 100644 --- a/prebuilt.json +++ b/prebuilt.json @@ -1 +1 @@ -{"protected_inventory_sha256": "b519174b880231cedd1dd6d1c29066db4523a5c72efe4317660eba7a9f340bd3", "schema_version": 1} +{"protected_inventory_sha256": "158cf8d45beb165a7fb5d2d5e2fc84c85b565fc7809078ecf6236c4acae21fee", "schema_version": 1} diff --git a/tools/ci/run_host_tests.py b/tools/ci/run_host_tests.py index 8d08a069f8..ff31f7b657 100644 --- a/tools/ci/run_host_tests.py +++ b/tools/ci/run_host_tests.py @@ -12,7 +12,8 @@ import tempfile ROOT = Path(__file__).resolve().parents[2] RUNNER = ROOT / 'tools/test_runner.py' -UI_PYTEST_FILES = ( +EXTRA_PYTEST_FILES = ( + 'openpilot/system/tests/test_sentry.py', 'openpilot/system/ui/lib/tests/test_text_caches.py', 'openpilot/system/ui/widgets/tests/test_label_cache.py', 'openpilot/selfdrive/ui/tests/test_onboarding_textures.py', @@ -28,7 +29,7 @@ def pytest_files(root=ROOT): (isinstance(node, ast.ClassDef) and node.name.startswith('Test') and not node.bases) for node in module.body): files.append(path.relative_to(root).as_posix()) - return files + [path for path in UI_PYTEST_FILES if (root / path).is_file()] + return files + [path for path in EXTRA_PYTEST_FILES if (root / path).is_file()] def main(arguments=None): diff --git a/tools/ci/tests/test_feature_test_discovery.py b/tools/ci/tests/test_feature_test_discovery.py index e400a829c1..f1bf792b5a 100644 --- a/tools/ci/tests/test_feature_test_discovery.py +++ b/tools/ci/tests/test_feature_test_discovery.py @@ -37,6 +37,7 @@ class TestFeatureTestDiscovery(unittest.TestCase): workflow = (ROOT / '.github/workflows/tests.yaml').read_text() self.assert_hosted_pytest_contract(workflow) selected = pytest_files() + self.assertIn('openpilot/system/tests/test_sentry.py', selected) self.assertTrue(selected) for package in ('flm', 'spot_monitor', 'sentry_mode', 'galaxy', 'ui'): self.assertTrue(any(path.startswith(f'openpilot/starpilot/{package}/') for path in selected), package)