fix(crashpad): route client logs through sentry logger#1859
Conversation
Forward Crashpad client log messages into the Sentry logger when debug logging is enabled, mapping severities between mini_chromium and Sentry levels. This makes detailed crashpad_handler startup errors, such as missing executable permissions, visible. Resolves: #792
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Fixes
- route client logs through sentry logger ([#1859](https://github.com/getsentry/sentry-native/pull/1859))If none of the above apply, you can opt out of this check by adding |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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.
Reviewed by Cursor Bugbot for commit 6a51748. Configure here.
| sentry__logger_log(to_sentry_level(severity), "crashpad: %.*s", | ||
| static_cast<int>(end - start), msg.c_str() + start); | ||
| return true; | ||
| }); |
There was a problem hiding this comment.
Unsafe logger use after fork
Medium Severity
The new Crashpad SetLogMessageHandler calls sentry__logger_log for every client log, including messages emitted in Crashpad’s forked intermediate process before exec. By then the SDK has already started its background worker, so this runs user logger callbacks plus non-async-signal-safe work such as malloc/stdio after a multithreaded fork, which can hang StartHandler or misbehave in custom loggers—on the same permission-denied path this change aims to surface.
Reviewed by Cursor Bugbot for commit 6a51748. Configure here.
There was a problem hiding this comment.
Ouch :( If we avoid unsafe Sentry logger callbacks from the forked intermediate process before exec, we lose exactly that one most important message that contains the information we want to make visible (permission denied).
[sentry] DEBUG starting backend
[sentry] DEBUG background worker thread started
[sentry] DEBUG starting crashpad backend with handler "/Users/jpnurmi/Projects/sentry/sentry-playground/build/crashpad_handler"
[sentry] DEBUG using minidump URL <url>
[64238:16278493:20260713,104941.870252:FATAL spawn_subprocess.cc:222] posix_spawnp /Users/jpnurmi/Projects/sentry/sentry-playground/build/crashpad_handler: Permission denied (13)
[sentry] WARN crashpad: intermediate process terminated by signal 5 (Trace/BPT trap: 5)
[sentry] ERROR crashpad: ReadExactly: expected 8, observed 0
[sentry] WARN failed to start crashpad client handler
[sentry] WARN failed to initialize backend
[sentry] WARN `sentry_init` failed
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| logging::SetLogMessageHandler( | ||
| [](logging::LogSeverity severity, const char *UNUSED(file), | ||
| int UNUSED(line), size_t start, const std::string &msg) { | ||
| size_t end = msg.size(); | ||
| if (end > start && msg[end - 1] == '\n') { | ||
| end--; | ||
| } | ||
| sentry__logger_log(to_sentry_level(severity), "crashpad: %.*s", | ||
| static_cast<int>(end - start), msg.c_str() + start); | ||
| return true; | ||
| }); |
There was a problem hiding this comment.
Bug: The log message handler unconditionally returns true, which suppresses the abort() call for LOG_FATAL messages, allowing the process to continue running after a fatal error.
Severity: HIGH
Suggested Fix
The log message handler should return false for LOG_FATAL messages. This will allow the default behavior, which includes calling abort(), to proceed and ensure the process terminates immediately upon a fatal error.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/backends/sentry_backend_crashpad.cpp#L787-L797
Potential issue: The log message handler unconditionally returns `true`. In
`mini_chromium`, if a log handler returns `true`, it prevents the default `abort()` call
for `LOG_FATAL` messages. This means that when a fatal error is logged, the process will
not terminate as expected. Instead, it will continue to execute in a potentially
inconsistent or corrupted state, which could lead to unpredictable behavior or silent
data corruption. This handler is installed globally in the parent process, affecting all
`LOG_FATAL` calls from crashpad client code.
Did we get this right? 👍 / 👎 to inform future reviews.
Update Crashpad so spawn and exec failures are reported from the parent process instead of logging from the forked intermediate child. This keeps handler startup diagnostics visible without invoking logger callbacks after fork. Co-Authored-By: OpenAI Codex <noreply@openai.com>
1 participant
Forward Crashpad client log messages to the Sentry logger instead of
stderr. This makes actionable Crashpad handler startup errors due to missing executable permissions, for example, visible in downstream SDKs that only capture Sentry logs, notstderr.Before:
After:
Resolves: #792