diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a0c88ff07..7d4b895583 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,32 +9,21 @@ **Internal**: - Updated Breakpad and Crashpad backends to 2022-09-14. ([#735](https://github.com/getsentry/sentry-native/pull/735)) +- Be more defensive around transactions ([#757](https://github.com/getsentry/sentry-native/pull/757)) ## 0.5.0 **Features** - Provide `on_crash()` callback to allow clients to act on detected crashes. - Users often inquired about distinguishing between crashes and "normal" events in the `before_send()` hook. - `on_crash()` can be considered a replacement for `before_send()` for crash events, where the goal is to use - `before_send()` only for normal events, while `on_crash()` is only invoked for crashes. This change is backward - compatible for current users of `before_send()` and allows gradual migration to `on_crash()` - ([see the docs for details](https://docs.sentry.io/platforms/native/configuration/filtering/)). - ([#724](https://github.com/getsentry/sentry-native/pull/724), - [#734](https://github.com/getsentry/sentry-native/pull/734)) + Users often inquired about distinguishing between crashes and "normal" events in the `before_send()` hook. `on_crash()` can be considered a replacement for `before_send()` for crash events, where the goal is to use `before_send()` only for normal events, while `on_crash()` is only invoked for crashes. This change is backward compatible for current users of `before_send()` and allows gradual migration to `on_crash()` ([see the docs for details](https://docs.sentry.io/platforms/native/configuration/filtering/)). ([#724](https://github.com/getsentry/sentry-native/pull/724), [#734](https://github.com/getsentry/sentry-native/pull/734)) **Fixes** -- Make Windows ModuleFinder more resilient to missing Debug Info - ([#732](https://github.com/getsentry/sentry-native/pull/732)) -- Aligned pre-send event processing in `sentry_capture_event()` with the - [cross-SDK session filter order](https://develop.sentry.dev/sdk/sessions/#filter-order) - ([#729](https://github.com/getsentry/sentry-native/pull/729)) -- Align the default value initialization for the `environment` payload attribute with the - [developer documentation](https://develop.sentry.dev/sdk/event-payloads/#optional-attribute) - ([#739](https://github.com/getsentry/sentry-native/pull/739)) -- Iterate all debug directory entries when parsing PE modules for a valid CodeView record - ([#740](https://github.com/getsentry/sentry-native/pull/740)) +- Make Windows ModuleFinder more resilient to missing Debug Info ([#732](https://github.com/getsentry/sentry-native/pull/732)) +- Aligned pre-send event processing in `sentry_capture_event()` with the [cross-SDK session filter order](https://develop.sentry.dev/sdk/sessions/#filter-order) ([#729](https://github.com/getsentry/sentry-native/pull/729)) +- Align the default value initialization for the `environment` payload attribute with the [developer documentation](https://develop.sentry.dev/sdk/event-payloads/#optional-attribute) ([#739](https://github.com/getsentry/sentry-native/pull/739)) +- Iterate all debug directory entries when parsing PE modules for a valid CodeView record ([#740](https://github.com/getsentry/sentry-native/pull/740)) **Thank you**: diff --git a/src/sentry_core.c b/src/sentry_core.c index 1a27fe06c9..669948d2d7 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -737,6 +737,10 @@ sentry_transaction_start( // traces_sampler. sentry_value_decref(sampling_ctx); + if (!opaque_tx_cxt) { + return NULL; + } + sentry_value_t tx_cxt = opaque_tx_cxt->inner; // If the parent span ID is some empty-ish value, just remove it @@ -1001,11 +1005,11 @@ sentry_span_finish(sentry_span_t *opaque_span) sentry_value_set_by_key(root_transaction, "spans", spans); } sentry_value_append(spans, span); - sentry__span_free(opaque_span); + sentry__span_decref(opaque_span); return; fail: - sentry__span_free(opaque_span); + sentry__span_decref(opaque_span); return; } diff --git a/src/sentry_tracing.c b/src/sentry_tracing.c index 49b607f1eb..9b9139a175 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -55,17 +55,13 @@ sentry_transaction_context_new(const char *name, const char *operation) if (!tx_cxt) { return NULL; } - memset(tx_cxt, 0, sizeof(sentry_transaction_context_t)); + tx_cxt->inner = sentry__value_transaction_context_new(name, operation); - sentry_value_t inner - = sentry__value_transaction_context_new(name, operation); - - if (sentry_value_is_null(inner)) { + if (sentry_value_is_null(tx_cxt->inner)) { + sentry_free(tx_cxt); return NULL; } - tx_cxt->inner = inner; - return tx_cxt; } @@ -87,36 +83,48 @@ void sentry_transaction_context_set_name( sentry_transaction_context_t *tx_cxt, const char *name) { - sentry_value_set_by_key( - tx_cxt->inner, "transaction", sentry_value_new_string(name)); + if (tx_cxt) { + sentry_value_set_by_key( + tx_cxt->inner, "transaction", sentry_value_new_string(name)); + } } void sentry_transaction_context_set_operation( sentry_transaction_context_t *tx_cxt, const char *operation) { - sentry_value_set_by_key( - tx_cxt->inner, "op", sentry_value_new_string(operation)); + if (tx_cxt) { + sentry_value_set_by_key( + tx_cxt->inner, "op", sentry_value_new_string(operation)); + } } void sentry_transaction_context_set_sampled( sentry_transaction_context_t *tx_cxt, int sampled) { - sentry_value_set_by_key( - tx_cxt->inner, "sampled", sentry_value_new_bool(sampled)); + if (tx_cxt) { + sentry_value_set_by_key( + tx_cxt->inner, "sampled", sentry_value_new_bool(sampled)); + } } void sentry_transaction_context_remove_sampled(sentry_transaction_context_t *tx_cxt) { - sentry_value_remove_by_key(tx_cxt->inner, "sampled"); + if (tx_cxt) { + sentry_value_remove_by_key(tx_cxt->inner, "sampled"); + } } void sentry_transaction_context_update_from_header( sentry_transaction_context_t *tx_cxt, const char *key, const char *value) { + if (!tx_cxt) { + return; + } + // do case-insensitive header key comparison const char sentry_trace[] = "sentry-trace"; for (size_t i = 0; i < sizeof(sentry_trace); i++) { @@ -169,7 +177,6 @@ sentry__transaction_new(sentry_value_t inner) if (!tx) { return NULL; } - memset(tx, 0, sizeof(sentry_transaction_t)); tx->inner = inner; @@ -179,7 +186,9 @@ sentry__transaction_new(sentry_value_t inner) void sentry__transaction_incref(sentry_transaction_t *tx) { - sentry_value_incref(tx->inner); + if (tx) { + sentry_value_incref(tx->inner); + } } void @@ -200,7 +209,9 @@ sentry__transaction_decref(sentry_transaction_t *tx) void sentry__span_incref(sentry_span_t *span) { - sentry_value_incref(span->inner); + if (span) { + sentry_value_incref(span->inner); + } } void @@ -212,6 +223,7 @@ sentry__span_decref(sentry_span_t *span) if (sentry_value_refcount(span->inner) <= 1) { sentry_value_decref(span->inner); + sentry__transaction_decref(span->transaction); sentry_free(span); } else { sentry_value_decref(span->inner); @@ -229,7 +241,6 @@ sentry__span_new(sentry_transaction_t *tx, sentry_value_t inner) if (!span) { return NULL; } - memset(span, 0, sizeof(sentry_span_t)); span->inner = inner; @@ -270,20 +281,6 @@ sentry__value_span_new( return sentry_value_new_null(); } -// TODO: for now, don't allow multiple references to spans. this should be -// revisited when sentry_transaction_t stores a list of sentry_span_t's -// instead of a list of sentry_value_t's. -void -sentry__span_free(sentry_span_t *span) -{ - if (!span) { - return; - } - sentry_value_decref(span->inner); - sentry__transaction_decref(span->transaction); - sentry_free(span); -} - sentry_value_t sentry__value_get_trace_context(sentry_value_t span) { @@ -323,8 +320,10 @@ sentry__value_get_trace_context(sentry_value_t span) void sentry_transaction_set_name(sentry_transaction_t *tx, const char *name) { - sentry_value_set_by_key( - tx->inner, "transaction", sentry_value_new_string(name)); + if (tx) { + sentry_value_set_by_key( + tx->inner, "transaction", sentry_value_new_string(name)); + } } static void @@ -348,13 +347,17 @@ void sentry_transaction_set_tag( sentry_transaction_t *tx, const char *tag, const char *value) { - set_tag(tx->inner, tag, value); + if (tx) { + set_tag(tx->inner, tag, value); + } } void sentry_span_set_tag(sentry_span_t *span, const char *tag, const char *value) { - set_tag(span->inner, tag, value); + if (span) { + set_tag(span->inner, tag, value); + } } static void @@ -369,13 +372,17 @@ remove_tag(sentry_value_t item, const char *tag) void sentry_transaction_remove_tag(sentry_transaction_t *tx, const char *tag) { - remove_tag(tx->inner, tag); + if (tx) { + remove_tag(tx->inner, tag); + } } void sentry_span_remove_tag(sentry_span_t *span, const char *tag) { - remove_tag(span->inner, tag); + if (span) { + remove_tag(span->inner, tag); + } } static void @@ -393,13 +400,17 @@ void sentry_transaction_set_data( sentry_transaction_t *tx, const char *key, sentry_value_t value) { - set_data(tx->inner, key, value); + if (tx) { + set_data(tx->inner, key, value); + } } void sentry_span_set_data(sentry_span_t *span, const char *key, sentry_value_t value) { - set_data(span->inner, key, value); + if (span) { + set_data(span->inner, key, value); + } } static void @@ -414,13 +425,17 @@ remove_data(sentry_value_t item, const char *key) void sentry_transaction_remove_data(sentry_transaction_t *tx, const char *key) { - remove_data(tx->inner, key); + if (tx) { + remove_data(tx->inner, key); + } } void sentry_span_remove_data(sentry_span_t *span, const char *key) { - remove_data(span->inner, key); + if (span) { + remove_data(span->inner, key); + } } sentry_value_t @@ -475,14 +490,18 @@ set_status(sentry_value_t item, sentry_span_status_t status) void sentry_span_set_status(sentry_span_t *span, sentry_span_status_t status) { - set_status(span->inner, status); + if (span) { + set_status(span->inner, status); + } } void sentry_transaction_set_status( sentry_transaction_t *tx, sentry_span_status_t status) { - set_status(tx->inner, status); + if (tx) { + set_status(tx->inner, status); + } } static void @@ -509,12 +528,16 @@ void sentry_span_iter_headers(sentry_span_t *span, sentry_iter_headers_function_t callback, void *userdata) { - sentry__span_iter_headers(span->inner, callback, userdata); + if (span) { + sentry__span_iter_headers(span->inner, callback, userdata); + } } void sentry_transaction_iter_headers(sentry_transaction_t *tx, sentry_iter_headers_function_t callback, void *userdata) { - sentry__span_iter_headers(tx->inner, callback, userdata); + if (tx) { + sentry__span_iter_headers(tx->inner, callback, userdata); + } } diff --git a/src/sentry_tracing.h b/src/sentry_tracing.h index bccc9bc78c..e04e5ee2fd 100644 --- a/src/sentry_tracing.h +++ b/src/sentry_tracing.h @@ -39,7 +39,6 @@ sentry_value_t sentry__value_span_new(size_t max_spans, sentry_value_t parent, char *operation, char *description); sentry_span_t *sentry__span_new( sentry_transaction_t *parent_tx, sentry_value_t inner); -void sentry__span_free(sentry_span_t *span); /** * Returns an object containing tracing information extracted from a diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index 052cf77956..a960273e03 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -773,7 +773,7 @@ SENTRY_TEST(distributed_headers) sentry_value_get_by_key(dist_tx->inner, "sampled"))); sentry__transaction_decref(dist_tx); - sentry__span_free(child); + sentry__span_decref(child); sentry__transaction_decref(tx); // check sampled flag @@ -801,7 +801,7 @@ SENTRY_TEST(distributed_headers) sentry_value_get_by_key(dist_tx->inner, "sampled"))); sentry__transaction_decref(dist_tx); - sentry__span_free(child); + sentry__span_decref(child); sentry__transaction_decref(tx); sentry_close();