From da0b0606c5fe0bb621b745f593b938ea5a0b0f95 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 27 Nov 2023 14:39:26 +0100 Subject: [PATCH 1/4] fix: Maintain client in crashpad state --- src/backends/sentry_backend_crashpad.cpp | 38 ++++++++++++++++-------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index 7a0a7d0d6..a4c4fea14 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -43,6 +43,14 @@ extern "C" { # pragma warning(pop) #endif +template +static void +safe_delete(T *&ptr) +{ + delete ptr; + ptr = nullptr; +} + extern "C" { #ifdef SENTRY_PLATFORM_LINUX @@ -74,6 +82,7 @@ constexpr int g_CrashSignals[] = { typedef struct { crashpad::CrashReportDatabase *db; + crashpad::CrashpadClient *client; sentry_path_t *event_path; sentry_path_t *breadcrumb1_path; sentry_path_t *breadcrumb2_path; @@ -81,6 +90,16 @@ typedef struct { sentry_value_t crash_event; } crashpad_state_t; +/** + * Correctly destruct C++ members of the crashpad state. + */ +static void +crashpad_state_dtor(crashpad_state_t *state) +{ + safe_delete(state->client); + safe_delete(state->db); +} + static void crashpad_backend_user_consent_changed(sentry_backend_t *backend) { @@ -325,24 +344,22 @@ crashpad_backend_startup( // Initialize database first, flushing the consent later on as part of // `sentry_init` will persist the upload flag. data->db = crashpad::CrashReportDatabase::Initialize(database).release(); - + data->client = new crashpad::CrashpadClient; bool success; - crashpad::CrashpadClient client; char *minidump_url = sentry__dsn_get_minidump_url(options->dsn, options->user_agent); if (minidump_url) { SENTRY_TRACEF("using minidump URL \"%s\"", minidump_url); - success = client.StartHandler(handler, database, database, minidump_url, - options->http_proxy ? options->http_proxy : "", annotations, - arguments, + success = data->client->StartHandler(handler, database, database, + minidump_url, options->http_proxy ? options->http_proxy : "", + annotations, arguments, /* restartable */ true, /* asynchronous_start */ false, attachments); sentry_free(minidump_url); } else { SENTRY_WARN( "failed to construct minidump URL (check DSN or user-agent)"); - delete data->db; - data->db = nullptr; + crashpad_state_dtor(data); return 1; } @@ -387,8 +404,7 @@ crashpad_backend_startup( } else { SENTRY_WARN("failed to start crashpad client handler"); // not calling `shutdown` - delete data->db; - data->db = nullptr; + crashpad_state_dtor(data); return 1; } @@ -432,9 +448,7 @@ crashpad_backend_shutdown(sentry_backend_t *backend) } #endif - auto *data = static_cast(backend->data); - delete data->db; - data->db = nullptr; + crashpad_state_dtor(static_cast(backend->data)); #ifdef SENTRY_PLATFORM_LINUX g_signal_stack.ss_flags = SS_DISABLE; From 93a3fd66291b0be0c48776d627ec535f75bedb43 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 27 Nov 2023 15:12:27 +0100 Subject: [PATCH 2/4] changelog --- CHANGELOG.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3786b9f66..f64877f42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,17 @@ # Changelog +## Unreleased + +**Fixes**: + +- Maintain crashpad client instance during Native SDK lifecycle. ([#910](https://github.com/getsentry/sentry-native/pull/910)) + ## 0.6.7 **Fixes**: -- Disable sigaltstack on Android ([#901](https://github.com/getsentry/sentry-native/pull/901)) -- Prevent stuck crashpad-client on Windows ([#902](https://github.com/getsentry/sentry-native/pull/902), [crashpad#89](https://github.com/getsentry/crashpad/pull/89)) +- Disable sigaltstack on Android. ([#901](https://github.com/getsentry/sentry-native/pull/901)) +- Prevent stuck crashpad-client on Windows. ([#902](https://github.com/getsentry/sentry-native/pull/902), [crashpad#89](https://github.com/getsentry/crashpad/pull/89)) ## 0.6.6 From 7f98f9e097b9cec8588719db6dedace1836d6930 Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 27 Nov 2023 19:21:49 +0100 Subject: [PATCH 3/4] Register WER module via state client --- src/backends/sentry_backend_crashpad.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index a4c4fea14..aa6b6f3a8 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -386,7 +386,7 @@ crashpad_backend_startup( SENTRY_WARN("registering crashpad WER handler in registry failed"); } else { std::wstring wer_path_string(wer_path->path); - if (!client.RegisterWerModule(wer_path_string)) { + if (!data->client.RegisterWerModule(wer_path_string)) { SENTRY_WARN("registering crashpad WER handler module failed"); } } From f07b3e3e4938fba24bcb359efc209342bfe72e5d Mon Sep 17 00:00:00 2001 From: Mischan Toosarani-Hausberger Date: Mon, 27 Nov 2023 20:02:10 +0100 Subject: [PATCH 4/4] typo --- src/backends/sentry_backend_crashpad.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index aa6b6f3a8..96d961657 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -386,7 +386,7 @@ crashpad_backend_startup( SENTRY_WARN("registering crashpad WER handler in registry failed"); } else { std::wstring wer_path_string(wer_path->path); - if (!data->client.RegisterWerModule(wer_path_string)) { + if (!data->client->RegisterWerModule(wer_path_string)) { SENTRY_WARN("registering crashpad WER handler module failed"); } }