From 50458beec64ba9f288f8295e479307b3dddb6d76 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Wed, 30 Jun 2021 16:27:01 +0200 Subject: [PATCH 1/2] fix: Make Crashpad Backend respect max_breadcrumbs setting --- src/backends/sentry_backend_crashpad.cpp | 11 ++++++----- src/sentry_backend.h | 3 ++- src/sentry_core.c | 3 ++- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index f92e62ddc..cf02958b2 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -324,16 +324,17 @@ sentry__crashpad_backend_shutdown(sentry_backend_t *backend) } static void -sentry__crashpad_backend_add_breadcrumb( - sentry_backend_t *backend, sentry_value_t breadcrumb) +sentry__crashpad_backend_add_breadcrumb(sentry_backend_t *backend, + sentry_value_t breadcrumb, const sentry_options_t *options) { crashpad_state_t *data = (crashpad_state_t *)backend->data; - bool first_breadcrumb = data->num_breadcrumbs % SENTRY_BREADCRUMBS_MAX == 0; + size_t max_breadcrumbs = options->max_breadcrumbs; + + bool first_breadcrumb = data->num_breadcrumbs % max_breadcrumbs == 0; const sentry_path_t *breadcrumb_file - = data->num_breadcrumbs % (SENTRY_BREADCRUMBS_MAX * 2) - < SENTRY_BREADCRUMBS_MAX + = data->num_breadcrumbs % (max_breadcrumbs * 2) < max_breadcrumbs ? data->breadcrumb1_path : data->breadcrumb2_path; data->num_breadcrumbs++; diff --git a/src/sentry_backend.h b/src/sentry_backend.h index 02faf3303..bb954272a 100644 --- a/src/sentry_backend.h +++ b/src/sentry_backend.h @@ -21,7 +21,8 @@ struct sentry_backend_s { sentry_backend_t *, const sentry_options_t *options); // NOTE: The breadcrumb is not moved into the hook and does not need to be // `decref`-d internally. - void (*add_breadcrumb_func)(sentry_backend_t *, sentry_value_t breadcrumb); + void (*add_breadcrumb_func)(sentry_backend_t *, sentry_value_t breadcrumb, + const sentry_options_t *options); void (*user_consent_changed_func)(sentry_backend_t *); uint64_t (*get_last_crash_func)(sentry_backend_t *); void *data; diff --git a/src/sentry_core.c b/src/sentry_core.c index 3774b99d8..ba1b54c69 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -480,7 +480,8 @@ sentry_add_breadcrumb(sentry_value_t breadcrumb) SENTRY_WITH_OPTIONS (options) { if (options->backend && options->backend->add_breadcrumb_func) { // the hook will *not* take ownership - options->backend->add_breadcrumb_func(options->backend, breadcrumb); + options->backend->add_breadcrumb_func( + options->backend, breadcrumb, options); } max_breadcrumbs = options->max_breadcrumbs; } From 8c4effbc2187b7c4291010568a4c3b838793ed61 Mon Sep 17 00:00:00 2001 From: Arpad Borsos Date: Thu, 1 Jul 2021 10:45:25 +0200 Subject: [PATCH 2/2] allow for max_breadcrumbs=0 --- src/backends/sentry_backend_crashpad.cpp | 3 +++ src/sentry_value.c | 12 +++++++++--- tests/unit/test_value.c | 9 +++++++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/backends/sentry_backend_crashpad.cpp b/src/backends/sentry_backend_crashpad.cpp index cf02958b2..dcebd9c7e 100644 --- a/src/backends/sentry_backend_crashpad.cpp +++ b/src/backends/sentry_backend_crashpad.cpp @@ -330,6 +330,9 @@ sentry__crashpad_backend_add_breadcrumb(sentry_backend_t *backend, crashpad_state_t *data = (crashpad_state_t *)backend->data; size_t max_breadcrumbs = options->max_breadcrumbs; + if (!max_breadcrumbs) { + return; + } bool first_breadcrumb = data->num_breadcrumbs % max_breadcrumbs == 0; diff --git a/src/sentry_value.c b/src/sentry_value.c index 668f0dc1e..0fb51df69 100644 --- a/src/sentry_value.c +++ b/src/sentry_value.c @@ -611,13 +611,19 @@ sentry__value_append_bounded(sentry_value_t value, sentry_value_t v, size_t max) // move 99 items (len - 1) // from 20 - size_t to_move = max - 1; + size_t to_move = max >= 1 ? max - 1 : 0; size_t to_shift = l->len - to_move; for (size_t i = 0; i < to_shift; i++) { sentry_value_decref(l->items[i]); } - memmove(l->items, l->items + (to_shift), to_move * sizeof(l->items[0])); - l->items[max - 1] = v; + if (to_move) { + memmove(l->items, l->items + to_shift, to_move * sizeof(l->items[0])); + } + if (max >= 1) { + l->items[max - 1] = v; + } else { + sentry_value_decref(v); + } l->len = max; return 0; diff --git a/tests/unit/test_value.c b/tests/unit/test_value.c index 1863241fc..783567f40 100644 --- a/tests/unit/test_value.c +++ b/tests/unit/test_value.c @@ -477,6 +477,15 @@ SENTRY_TEST(value_collections_leak) TEST_CHECK_INT_EQUAL(sentry_value_refcount(obj), 3); + sentry_value_incref(obj); + sentry__value_append_bounded(list, obj, 1); + TEST_CHECK_INT_EQUAL(sentry_value_refcount(obj), 2); + + sentry_value_incref(obj); + sentry__value_append_bounded(list, obj, 0); + TEST_CHECK_INT_EQUAL(sentry_value_refcount(obj), 1); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(list), 0); + sentry_value_decref(list); TEST_CHECK_INT_EQUAL(sentry_value_refcount(obj), 1);