Skip to content

fix(native): external crash reporter#1589

Merged
jpnurmi merged 6 commits into
masterfrom
jpnurmi/fix/native-external-crash-reporter
Mar 24, 2026
Merged

fix(native): external crash reporter#1589
jpnurmi merged 6 commits into
masterfrom
jpnurmi/fix/native-external-crash-reporter

Conversation

@jpnurmi

@jpnurmi jpnurmi commented Mar 23, 2026

Copy link
Copy Markdown
Collaborator

The native backend's crash daemon now invokes sentry__launch_external_crash_reporter() after processing a crash, writing the envelope to the external directory and spawning the configured reporter process. When an external reporter is configured, the daemon does not send envelopes via its own transport.

Refactored sentry__launch_external_crash_reporter() to accept options directly instead of using SENTRY_WITH_OPTIONS (which relies on global state unavailable in the daemon process), and to use sentry__transport_send_envelope() directly to bypass the consent check in sentry__capture_envelope() since upload consent is the external reporter's concern.

Also adds event_id to the envelope headers written by the daemon's crash processing functions, and removes the incomplete session-only envelope that native_backend_flush_scope was writing to the external directory.

image

jpnurmi and others added 3 commits March 23, 2026 12:09
The native backend's crash daemon now invokes `sentry__launch_external_crash_reporter()`
after processing a crash, writing the envelope to the external directory and spawning
the configured reporter process. When an external reporter is configured, the daemon
does not send envelopes via its own transport.

Refactored `sentry__launch_external_crash_reporter()` to accept `options` directly
instead of using `SENTRY_WITH_OPTIONS` (which relies on global state unavailable in
the daemon process), and to use `sentry__transport_send_envelope()` directly to bypass
the consent check in `sentry__capture_envelope()` since upload consent is the external
reporter's concern.

Also adds event_id to the envelope headers written by the daemon's crash processing
functions, and removes the incomplete session-only envelope that `native_backend_flush_scope`
was writing to the external directory.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jpnurmi jpnurmi force-pushed the jpnurmi/fix/native-external-crash-reporter branch from 05a8064 to a626e07 Compare March 23, 2026 11:42
@jpnurmi jpnurmi marked this pull request as ready for review March 23, 2026 12:26
Comment thread src/backends/native/sentry_crash_daemon.c Outdated
Comment thread src/backends/native/sentry_crash_daemon.c

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Double-free of envelope after external crash reporter
    • I removed the daemon-side free in the external-crash-reporter success path so envelope ownership remains with the external disk transport.
  • ✅ Fixed: Memory leak of event_json on error path
    • I now free event_json when envelope file open fails and unconditionally after event writing so all paths release the buffer.

Create PR

Or push these changes by commenting:

@cursor push e794f41a23
Preview (e794f41a23)
diff --git a/src/backends/native/sentry_crash_daemon.c b/src/backends/native/sentry_crash_daemon.c
--- a/src/backends/native/sentry_crash_daemon.c
+++ b/src/backends/native/sentry_crash_daemon.c
@@ -2280,6 +2280,7 @@
     if (fd < 0) {
         SENTRY_WARN("Failed to open envelope file for writing");
         sentry_free(event_id);
+        sentry_free(event_json);
         return false;
     }
 
@@ -2333,8 +2334,8 @@
             _write(fd, "\n", 1);
 #endif
         }
-        sentry_free(event_json);
     }
+    sentry_free(event_json);
 
     // Add minidump as attachment
 #if defined(SENTRY_PLATFORM_UNIX)
@@ -2750,8 +2751,6 @@
             SENTRY_WARN("No transport available for sending envelope");
             sentry_envelope_free(envelope);
         }
-    } else {
-        sentry_envelope_free(envelope);
     }
 
     // Clean up temporary envelope file (keep minidump for

This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

Comment thread src/backends/native/sentry_crash_daemon.c
Comment thread src/backends/native/sentry_crash_daemon.c
Comment thread src/backends/native/sentry_crash_daemon.c

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment thread src/sentry_core.c

@JoshuaMoelans JoshuaMoelans left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@jpnurmi jpnurmi merged commit 231d9a6 into master Mar 24, 2026
85 of 87 checks passed
@jpnurmi jpnurmi deleted the jpnurmi/fix/native-external-crash-reporter branch March 24, 2026 08:02
BernhardMarconato pushed a commit to elgatosf/sentry-native that referenced this pull request Apr 21, 2026
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants