Open
Conversation
Contributor
mujacica
commented
Oct 29, 2025
mujacica
commented
- Sentry native crash backend
- Out-of-process daemon/handler
- MacOS/Android/Windows/Linux support
- Integration with external crash reporter
- IPC/SHM/Signaling multi-platform implementation
- Minidump writers for all platforms
- In-process option (process_crash function)
- Different options for Minidump sizes
- Full sentry-codebase integration
- Sentry logger integration
- Sentry debug-flags integration
Contributor
Author
|
@sentry review |
1 similar comment
Contributor
Author
|
@sentry review |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
mujacica
force-pushed
the
feat/sentry_native_backend
branch
from
216b3cd to
e4cd98c
Compare
Contributor
Author
|
@sentry review |
Contributor
Author
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Author
|
@cursor review |
mujacica
force-pushed
the
feat/sentry_native_backend
branch
from
d634773 to
eb77775
Compare
Contributor
Author
|
@sentry review |
mujacica
force-pushed
the
feat/sentry_native_backend
branch
from
d46d50a to
d38b5e1
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
mujacica
force-pushed
the
feat/sentry_native_backend
branch
from
d33258f to
71ea60d
Compare
mujacica
changed the title
|
mujacica
force-pushed
the
feat/sentry_native_backend
branch
from
79d2b05 to
a6af7d6
Compare
- Extract duplicated helper functions from Linux/macOS minidump writers into new common files (sentry_minidump_common.c/h) - Fix Windows OpenProcess to use minimal required permissions (PROCESS_QUERY_INFORMATION | PROCESS_VM_READ) instead of PROCESS_ALL_ACCESS - Add validation to skip setting debug_id for modules without valid PDB info (all-zero UUID) to fix Windows symbolication issues Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add length validation in sentry__minidump_write_string to prevent integer overflow when calculating UTF-16 buffer size - Remove unused struct linux_fxsave from Linux minidump writer - Remove unused SENTRY_CRASH_PID_STRING_SIZE constant Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The crashed thread's stacktrace was appearing both in exception.values[0].stacktrace AND threads.values[0].stacktrace. This duplication could confuse Sentry's symbolicator. Fix: Only add stacktrace to non-crashed threads in the threads array, since the crashed thread's stacktrace is already in the exception. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add defensive uppercase conversion for Windows PE code_id formatting. While %lX should produce uppercase, some runtime environments may differ. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Cast e_shentsize to size_t before multiplication to prevent integer overflow. Both operands are uint16_t which promotes to int, and the maximum product (65535 * 65535) exceeds INT32_MAX. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace strlen() and snprintf() with signal-safe alternatives: - Add safe_strlen() function for all Unix platforms (moved from macOS-only) - Add safe_uint_to_str() for signal-safe integer to string conversion - Add safe_build_stack_path() for signal-safe path building - Replace strlen() with sizeof()-1 for string literals in signal handler - Replace snprintf() with manual string building using signal-safe helpers These functions avoid calling async-signal-unsafe libc functions from within the crash signal handler, preventing potential deadlocks or corruption during crash handling. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Windows exception codes like 0xC0000005 (Access Violation) are unsigned 32-bit values. When cast to int32, values >= 0x80000000 become negative, which causes symbolicator to fail with "invalid value: integer -1073741819, expected u32". Use sentry_value_new_uint64() for Windows exception codes to preserve the unsigned value. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The previous implementation performed byte-by-byte conversion which only worked for ASCII. Multi-byte UTF-8 sequences (e.g., "é" = 0xC3 0xA9) were incorrectly converted to separate UTF-16 code units (0x00C3 0x00A9 instead of 0x00E9), producing garbled text in module names. Implement proper UTF-8 decoding that: - Handles 1-4 byte UTF-8 sequences correctly - Produces UTF-16 surrogate pairs for code points > U+FFFF - Uses replacement character (U+FFFD) for invalid sequences - Rejects overlong encodings and invalid surrogates This fixes symbolication for users with international characters in paths. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add defensive deduplication check when enumerating threads from the crashed process on Windows. This prevents duplicate thread entries in the event JSON if CreateToolhelp32Snapshot returns duplicates or if there's a race condition during thread enumeration. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Previously, enumerate_threads_from_process only stored thread IDs but set CONTEXT to zeros for non-crashed threads. This meant we couldn't walk their stacks, resulting in threads with no labels in the Sentry UI (labels come from symbolicated top stack frames). Now we: - Open each thread with THREAD_GET_CONTEXT access - Suspend the thread briefly - Capture the full CPU context via GetThreadContext - Resume the thread This allows the daemon to walk all thread stacks using StackWalk64, providing proper stack traces that can be symbolicated to show meaningful thread labels. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add secondary deduplication when building thread list for event JSON - Verify crashed thread at index 0 matches crashed_tid before enumeration - Fix mismatch if threads[0].thread_id differs from crashed_tid - Add debug logging to track thread enumeration - Log warnings when duplicate thread IDs are detected and skipped Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The shared memory was being zeroed twice in the Linux IPC initialization when !shm_exists: once at line 124 and again at line 162. The first memset was completely redundant since the second one immediately overwrites the same memory region before initializing magic, version, and state fields. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
mujacica
force-pushed
the
feat/sentry_native_backend
branch
from
e9c32f5 to
d75958d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
- Remove defensive thread deduplication at event-building level - Remove thread ID mismatch check/fix - Add diagnostic logging at key points: - enumerate_threads start (shows crashed_tid and threads[0].id) - write_envelope_with_native_stacktrace (shows minidump_path and include_threads) - Windows thread addition (shows thread count being added) The duplicate thread issue needs proper root cause analysis. These logs will help identify whether: - Threads are duplicated in ctx->platform.threads[] - The threads are being added to the event twice - include_threads logic has issues Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Check lseek return value when saving position in module list stream - Check lseek return value when restoring position after module update - Fix SENTRY_DEBUGF formatting to comply with clang-format Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add detailed logging to identify where thread duplication occurs: - Log whether event JSON contains threads (and count) - Log mode, use_native_mode, need_minidump at decision point - Log minidump_path being passed to write_envelope_with_native_stacktrace - Warn if threads are in event when include_threads should be false This will help determine if duplication is in our code or server-side. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add verify_no_thread_duplication() to detect when threads appear twice
- Add print_daemon_logs() to output daemon log files for debugging
- Call thread verification in all E2E crash mode tests
- Print daemon logs after each crash for Windows thread investigation
The daemon already logs to {database_path}/sentry-daemon-{id}.log when
debug mode is enabled. This change makes those logs visible in CI output.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Debug output to understand what API returns: - Print all thread IDs and properties - Detect if multiple thread entries exist in API response - Help diagnose thread duplication issue on Windows Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.