From fe461ad077399caebb63d612bafd2c8492346c2f Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 15:18:29 +0200 Subject: [PATCH 01/20] fix(tracing): finish active trace on crash Co-authored-by: Claude Opus 4.7 (1M context) --- examples/example.c | 15 +++++- src/backends/sentry_backend_breakpad.cpp | 2 + src/backends/sentry_backend_inproc.c | 2 + src/backends/sentry_backend_native.c | 2 + src/sentry_tracing.c | 30 ++++++++++++ src/sentry_tracing.h | 7 +++ tests/test_integration_http.py | 61 ++++++++++++++++++++++++ tests/unit/test_tracing.c | 54 +++++++++++++++++++++ tests/unit/tests.inc | 1 + 9 files changed, 173 insertions(+), 1 deletion(-) diff --git a/examples/example.c b/examples/example.c index 1c5ef292b..95514a53c 100644 --- a/examples/example.c +++ b/examples/example.c @@ -596,7 +596,8 @@ main(int argc, char **argv) options, sentry_transport_new(print_envelope)); } - if (has_arg(argc, argv, "capture-transaction")) { + if (has_arg(argc, argv, "capture-transaction") + || has_arg(argc, argv, "open-transaction")) { sentry_options_set_traces_sample_rate(options, 1.0); } @@ -1048,6 +1049,18 @@ main(int argc, char **argv) fflush(stdout); } + if (has_arg(argc, argv, "open-transaction")) { + // Start a transaction + child span on the scope and intentionally + // do not finish them. On a subsequent crash, the backend's auto- + // finalize is expected to ship the transaction envelope. + sentry_transaction_context_t *otx_ctx + = sentry_transaction_context_new("open.tx", "op"); + sentry_transaction_t *otx + = sentry_transaction_start(otx_ctx, sentry_value_new_null()); + sentry_set_transaction_object(otx); + sentry_set_span(sentry_transaction_start_child(otx, "open.span", NULL)); + } + if (has_arg(argc, argv, "crash")) { trigger_crash(); } diff --git a/src/backends/sentry_backend_breakpad.cpp b/src/backends/sentry_backend_breakpad.cpp index de10c55e1..bf1fb44b3 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -139,6 +139,8 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, SENTRY_WITH_OPTIONS (options) { sentry__write_crash_marker(options); + sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); + bool should_handle = true; if (options->on_crash_func) { diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index f3f9e0e48..f23e3cce8 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -1115,6 +1115,8 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx, bool should_handle = true; sentry__write_crash_marker(options); + sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); + if (options->on_crash_func && !skip_hooks) { SENTRY_DEBUG("invoking `on_crash` hook"); event = options->on_crash_func(uctx, event, options->on_crash_data); diff --git a/src/backends/sentry_backend_native.c b/src/backends/sentry_backend_native.c index fe1af9322..350d4a7d1 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -988,6 +988,8 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx) // Write crash marker sentry__write_crash_marker(options); + sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); + // Create crash event sentry_value_t event = sentry_value_new_event(); sentry_value_set_by_key( diff --git a/src/sentry_tracing.c b/src/sentry_tracing.c index a5f69e1ce..fb9742d64 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -967,3 +967,33 @@ sentry_transaction_iter_headers(sentry_transaction_t *tx, sentry__span_iter_headers(tx->inner, callback, userdata); } } + +void +sentry__trace_finish(sentry_span_status_t status) +{ + sentry_span_t *active_span = NULL; + sentry_transaction_t *active_tx = NULL; + SENTRY_WITH_SCOPE_MUT (scope) { + if (scope->span) { + sentry__span_incref(scope->span); + active_span = scope->span; + // sentry_set_span clears scope->transaction_object; the tx + // is reachable only via the span. + if (active_span->transaction) { + sentry__transaction_incref(active_span->transaction); + active_tx = active_span->transaction; + } + } else if (scope->transaction_object) { + sentry__transaction_incref(scope->transaction_object); + active_tx = scope->transaction_object; + } + } + if (active_span) { + sentry_span_set_status(active_span, status); + sentry_span_finish(active_span); + } + if (active_tx) { + sentry_transaction_set_status(active_tx, status); + sentry_transaction_finish(active_tx); + } +} diff --git a/src/sentry_tracing.h b/src/sentry_tracing.h index d8910ba7f..935351f2d 100644 --- a/src/sentry_tracing.h +++ b/src/sentry_tracing.h @@ -41,6 +41,13 @@ sentry_transaction_t *sentry__transaction_new(sentry_value_t inner); void sentry__transaction_incref(sentry_transaction_t *tx); void sentry__transaction_decref(sentry_transaction_t *tx); +/** + * Finishes the active span/transaction (if any) with `status` and copies + * their trace IDs to the propagation context, so a subsequently-built + * crash event nests under the active span. No-op if nothing is active. + */ +void sentry__trace_finish(sentry_span_status_t status); + void sentry__span_incref(sentry_span_t *span); void sentry__span_decref(sentry_span_t *span); diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 0b3352665..be6fc9c50 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -854,6 +854,67 @@ def test_native_crash_http(cmake, httpserver): assert_attachment(envelope) +@pytest.mark.parametrize( + "backend", + [ + "inproc", + pytest.param( + "breakpad", + marks=pytest.mark.skipif( + not has_breakpad or is_qemu, reason="test needs breakpad backend" + ), + ), + pytest.param( + "native", + marks=pytest.mark.skipif( + not has_native or is_qemu or is_kcov, + reason="test needs native backend", + ), + ), + ], +) +def test_trace_finish_on_crash(cmake, httpserver, backend): + """The backend's crash handler calls `sentry__trace_finish`, so an + unfinished transaction on the scope ships alongside the crash.""" + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": backend}) + + # Expect the crash envelope + the auto-finalized tx envelope. + httpserver.expect_oneshot_request( + "/api/123456/envelope/", + headers={"x-sentry-auth": auth_header}, + ).respond_with_data("OK") + httpserver.expect_oneshot_request( + "/api/123456/envelope/", + headers={"x-sentry-auth": auth_header}, + ).respond_with_data("OK") + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) + + with httpserver.wait(timeout=10) as waiting: + run( + tmp_path, + "sentry_example", + ["log", "open-transaction", "crash"], + expect_failure=True, + env=env, + ) + if backend != "native": + # inproc/breakpad cache to disk; the next launch ships them. + run(tmp_path, "sentry_example", ["log", "no-setup"], env=env) + assert waiting.result + + tx_items = [ + item + for req, _ in httpserver.log + for item in Envelope.deserialize(req.get_data()).items + if item.headers.get("type") == "transaction" + ] + assert tx_items + + tx = tx_items[0].payload.json + assert tx["contexts"]["trace"]["status"] == "aborted" + assert any(s.get("op") == "open.span" for s in tx.get("spans", [])) + + @pytest.mark.skipif(not has_files, reason="test needs a local filesystem") def test_http_retry_on_network_error(cmake, httpserver, unreachable_dsn): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "inproc"}) diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index 9a32a0167..67b9d5dee 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -778,6 +778,60 @@ check_spans(sentry_envelope_t *envelope, void *data) sentry_envelope_free(envelope); } +static void +check_aborted_transaction(sentry_envelope_t *envelope, void *data) +{ + uint64_t *called = data; + *called += 1; + + sentry_value_t tx = sentry_envelope_get_transaction(envelope); + TEST_CHECK(!sentry_value_is_null(tx)); + CHECK_STRING_PROPERTY(sentry_value_get_by_key( + sentry_value_get_by_key(tx, "contexts"), "trace"), + "status", "aborted"); + + sentry_envelope_free(envelope); +} + +SENTRY_TEST(trace_finish) +{ + // No active span/tx: no-op, no crash. + sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); + + uint64_t called = 0; + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_options_set_auto_session_tracking(options, 0); + + sentry_transport_t *transport + = sentry_transport_new(check_aborted_transaction); + sentry_transport_set_state(transport, &called); + sentry_options_set_transport(options, transport); + sentry_options_set_traces_sample_rate(options, 1.0); + sentry_init(options); + + sentry_transaction_context_t *ctx + = sentry_transaction_context_new("root", "op"); + sentry_transaction_t *tx + = sentry_transaction_start(ctx, sentry_value_new_null()); + sentry_set_transaction_object(tx); + + sentry_set_span(sentry_transaction_start_child(tx, "child-op", "child")); + + sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); + + // Scope is torn down: no active span or tx remains. + SENTRY_WITH_SCOPE (scope) { + TEST_CHECK(scope->span == NULL); + TEST_CHECK(scope->transaction_object == NULL); + } + + sentry_close(); + + // The tx envelope was sent with status=aborted. + TEST_CHECK_INT_EQUAL(called, 1); +} + SENTRY_TEST(drop_unfinished_spans) { uint64_t called_transport = 0; diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index d673a13e9..b3099dab0 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -308,6 +308,7 @@ XX(symbolizer) XX(task_queue) XX(thread_without_name_still_valid) XX(trace_continuation_truth_table) +XX(trace_finish) XX(traceparent_header_disabled_by_default) XX(traceparent_header_generation) XX(transaction_name_backfill_on_finish) From 102fe31f994ab1443749720e631f39a7d72ec122 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 17:55:55 +0200 Subject: [PATCH 02/20] fix(tracing): finish all in-flight children on crash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prior crash-path trace finish only closed `scope->span` (the deepest active span) and its root transaction. Intermediate spans between them — e.g. Qt event dispatch handlers nested above a crashing slot — were never finished, never added to `tx.spans`, and ended up orphaning the crash event at the trace root in Sentry's UI. Track every span on its root transaction under `children_mutex` at `sentry__span_new`, deregister on normal finish, and drain the list in leaf-first order inside `sentry__trace_finish`. Matches sentry-cocoa's `SentryTracer` `_children` + `finishForCrash` and sentry-java's `SentryTracer.forceFinish`. Preserve `scope->span` / `scope->transaction_object` across the drain so the subsequent crash event inherits the full trace context from scope (mirrors cocoa's `finishTracer:shouldCleanUp:NO`). Without this, the crash event fell through to the stale propagation context and Sentry could not nest it under the active span chain. Co-Authored-By: Claude Opus 4.7 (1M context) --- examples/example.c | 12 ++- src/backends/sentry_backend_breakpad.cpp | 1 + src/backends/sentry_backend_inproc.c | 1 + src/backends/sentry_backend_native.c | 1 + src/sentry_core.c | 2 + src/sentry_sampling_context.h | 5 +- src/sentry_tracing.c | 131 ++++++++++++++++++++--- src/sentry_tracing.h | 23 +++- tests/test_integration_http.py | 25 ++++- tests/unit/test_tracing.c | 24 ++++- 10 files changed, 194 insertions(+), 31 deletions(-) diff --git a/examples/example.c b/examples/example.c index 95514a53c..5a7571ffb 100644 --- a/examples/example.c +++ b/examples/example.c @@ -1050,15 +1050,19 @@ main(int argc, char **argv) } if (has_arg(argc, argv, "open-transaction")) { - // Start a transaction + child span on the scope and intentionally - // do not finish them. On a subsequent crash, the backend's auto- - // finalize is expected to ship the transaction envelope. + // Start a transaction + nested child spans on the scope and + // intentionally do not finish them. On a subsequent crash, the + // backend's auto-finalize is expected to ship the transaction envelope + // with all the in-flight children closed out, not just the deepest. sentry_transaction_context_t *otx_ctx = sentry_transaction_context_new("open.tx", "op"); sentry_transaction_t *otx = sentry_transaction_start(otx_ctx, sentry_value_new_null()); sentry_set_transaction_object(otx); - sentry_set_span(sentry_transaction_start_child(otx, "open.span", NULL)); + sentry_span_t *ospan + = sentry_transaction_start_child(otx, "open.span", NULL); + sentry_set_span( + sentry_span_start_child(ospan, "open.grand.span", NULL)); } if (has_arg(argc, argv, "crash")) { diff --git a/src/backends/sentry_backend_breakpad.cpp b/src/backends/sentry_backend_breakpad.cpp index bf1fb44b3..49f4205b5 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -20,6 +20,7 @@ extern "C" { #include "sentry_session_replay.h" #include "sentry_string.h" #include "sentry_sync.h" +#include "sentry_tracing.h" #include "sentry_transport.h" #include "sentry_unix_pageallocator.h" #include "transports/sentry_disk_transport.h" diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index f23e3cce8..0a7585e0c 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -18,6 +18,7 @@ #include "sentry_screenshot.h" #include "sentry_session_replay.h" #include "sentry_sync.h" +#include "sentry_tracing.h" #include "sentry_transport.h" #include "sentry_unix_pageallocator.h" #include "transports/sentry_disk_transport.h" diff --git a/src/backends/sentry_backend_native.c b/src/backends/sentry_backend_native.c index 350d4a7d1..49e634fe4 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -37,6 +37,7 @@ #include "sentry_scope.h" #include "sentry_session.h" #include "sentry_sync.h" +#include "sentry_tracing.h" #include "sentry_transport.h" #include "transports/sentry_disk_transport.h" diff --git a/src/sentry_core.c b/src/sentry_core.c index 3019f7884..140084d0a 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -1569,6 +1569,8 @@ sentry_span_finish_ts(sentry_span_t *opaque_span, uint64_t timestamp) goto fail; } + sentry__transaction_remove_child(opaque_root_transaction, opaque_span); + sentry_value_t root_transaction = opaque_root_transaction->inner; if (!sentry_value_is_true( diff --git a/src/sentry_sampling_context.h b/src/sentry_sampling_context.h index 59a069258..6977f52cb 100644 --- a/src/sentry_sampling_context.h +++ b/src/sentry_sampling_context.h @@ -2,10 +2,11 @@ #ifndef SENTRY_SAMPLING_CONTEXT_H_INCLUDED #define SENTRY_SAMPLING_CONTEXT_H_INCLUDED -#include "sentry_tracing.h" +#include "sentry_boot.h" +#include "sentry_value.h" typedef struct sentry_sampling_context_s { - sentry_transaction_context_t *transaction_context; + struct sentry_transaction_context_s *transaction_context; sentry_value_t custom_sampling_context; bool *parent_sampled; double sample_rand; diff --git a/src/sentry_tracing.c b/src/sentry_tracing.c index fb9742d64..a1ce80ebb 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -452,6 +452,10 @@ sentry__transaction_new(sentry_value_t inner) } tx->inner = inner; + sentry__mutex_init(&tx->children_mutex); + tx->children = NULL; + tx->children_count = 0; + tx->children_cap = 0; return tx; } @@ -473,12 +477,38 @@ sentry__transaction_decref(sentry_transaction_t *tx) if (sentry_value_refcount(tx->inner) <= 1) { sentry_value_decref(tx->inner); + for (size_t i = 0; i < tx->children_count; i++) { + sentry__span_decref(tx->children[i]); + } + sentry_free(tx->children); + sentry__mutex_free(&tx->children_mutex); sentry_free(tx); } else { sentry_value_decref(tx->inner); } } +void +sentry__transaction_remove_child(sentry_transaction_t *tx, sentry_span_t *span) +{ + if (!tx || !span) { + return; + } + bool found = false; + sentry__mutex_lock(&tx->children_mutex); + for (size_t i = 0; i < tx->children_count; i++) { + if (tx->children[i] == span) { + tx->children[i] = tx->children[--tx->children_count]; + found = true; + break; + } + } + sentry__mutex_unlock(&tx->children_mutex); + if (found) { + sentry__span_decref(span); + } +} + void sentry__span_incref(sentry_span_t *span) { @@ -520,6 +550,27 @@ sentry__span_new(sentry_transaction_t *tx, sentry_value_t inner) sentry__transaction_incref(tx); span->transaction = tx; + sentry__mutex_lock(&tx->children_mutex); + if (tx->children_count == tx->children_cap) { + size_t new_cap = tx->children_cap ? tx->children_cap * 2 : 4; + sentry_span_t **new_children + = sentry_malloc(new_cap * sizeof(sentry_span_t *)); + if (new_children) { + if (tx->children) { + memcpy(new_children, tx->children, + tx->children_count * sizeof(sentry_span_t *)); + sentry_free(tx->children); + } + tx->children = new_children; + tx->children_cap = new_cap; + } + } + if (tx->children_count < tx->children_cap) { + sentry__span_incref(span); + tx->children[tx->children_count++] = span; + } + sentry__mutex_unlock(&tx->children_mutex); + return span; } @@ -971,29 +1022,75 @@ sentry_transaction_iter_headers(sentry_transaction_t *tx, void sentry__trace_finish(sentry_span_status_t status) { - sentry_span_t *active_span = NULL; + // Save `scope->span` / `scope->transaction_object` so the subsequent crash + // event still sees the active trace context (matches sentry-cocoa's + // `finishTracer:shouldCleanUp:NO`). Finishing the tx through the normal + // path clears both; we restore them after the drain so the finished spans + // — which now carry `timestamp` but otherwise retain trace_id, span_id, + // parent_span_id, op, etc. — stay on scope for event capture. + sentry_span_t *saved_span = NULL; + sentry_transaction_t *saved_tx_obj = NULL; sentry_transaction_t *active_tx = NULL; - SENTRY_WITH_SCOPE_MUT (scope) { + SENTRY_WITH_SCOPE (scope) { if (scope->span) { sentry__span_incref(scope->span); - active_span = scope->span; - // sentry_set_span clears scope->transaction_object; the tx - // is reachable only via the span. - if (active_span->transaction) { - sentry__transaction_incref(active_span->transaction); - active_tx = active_span->transaction; - } - } else if (scope->transaction_object) { + saved_span = scope->span; + } + if (scope->transaction_object) { sentry__transaction_incref(scope->transaction_object); - active_tx = scope->transaction_object; + saved_tx_obj = scope->transaction_object; + } + if (saved_span && saved_span->transaction) { + sentry__transaction_incref(saved_span->transaction); + active_tx = saved_span->transaction; + } else if (saved_tx_obj) { + sentry__transaction_incref(saved_tx_obj); + active_tx = saved_tx_obj; } } - if (active_span) { - sentry_span_set_status(active_span, status); - sentry_span_finish(active_span); + if (!active_tx) { + sentry__span_decref(saved_span); + sentry__transaction_decref(saved_tx_obj); + return; } - if (active_tx) { - sentry_transaction_set_status(active_tx, status); - sentry_transaction_finish(active_tx); + + // Swap out the live-children list so `sentry_span_finish_ts`'s remove path + // becomes a no-op for the drained spans; iterate in reverse (leaf-first) + // so `scope->span` is cleared before its ancestors are finished. + sentry__mutex_lock(&active_tx->children_mutex); + sentry_span_t **drained = active_tx->children; + size_t drained_count = active_tx->children_count; + active_tx->children = NULL; + active_tx->children_count = 0; + active_tx->children_cap = 0; + sentry__mutex_unlock(&active_tx->children_mutex); + + uint64_t end_ts = sentry__usec_time(); + for (size_t i = drained_count; i-- > 0;) { + sentry_span_t *child = drained[i]; + sentry_span_set_status(child, status); + sentry_span_finish_ts(child, end_ts); + sentry__span_decref(child); + } + sentry_free(drained); + + sentry_transaction_set_status(active_tx, status); + sentry_transaction_finish_ts(active_tx, end_ts); + + // Restore scope so `sentry__apply_scope_to_event` (called when the crash + // event is captured right after) picks up the full trace context off the + // saved span/tx rather than falling through to the stale propagation + // context. + SENTRY_WITH_SCOPE_MUT (scope) { + if (!scope->span && saved_span) { + scope->span = saved_span; + saved_span = NULL; + } + if (!scope->transaction_object && saved_tx_obj) { + scope->transaction_object = saved_tx_obj; + saved_tx_obj = NULL; + } } + sentry__span_decref(saved_span); + sentry__transaction_decref(saved_tx_obj); } diff --git a/src/sentry_tracing.h b/src/sentry_tracing.h index 935351f2d..0d9599c9e 100644 --- a/src/sentry_tracing.h +++ b/src/sentry_tracing.h @@ -2,6 +2,7 @@ #define SENTRY_TRACING_H_INCLUDED #include "sentry_slice.h" +#include "sentry_sync.h" #include "sentry_value.h" // W3C traceparent header: 00--- @@ -33,6 +34,13 @@ struct sentry_transaction_context_s { */ struct sentry_transaction_s { sentry_value_t inner; + // Live (unfinished) child spans, so `sentry__trace_finish` can close them + // out on crash. Entries are added in `sentry__span_new` and removed in + // `sentry_span_finish_ts`. Each entry holds one ref on the span. + sentry_mutex_t children_mutex; + sentry_span_t **children; + size_t children_count; + size_t children_cap; }; void sentry__transaction_context_free(sentry_transaction_context_t *tx_ctx); @@ -42,9 +50,18 @@ void sentry__transaction_incref(sentry_transaction_t *tx); void sentry__transaction_decref(sentry_transaction_t *tx); /** - * Finishes the active span/transaction (if any) with `status` and copies - * their trace IDs to the propagation context, so a subsequently-built - * crash event nests under the active span. No-op if nothing is active. + * Removes `span` from the transaction's live-children list and drops the ref + * that was taken by `sentry__span_new`. No-op if not found. + */ +void sentry__transaction_remove_child( + sentry_transaction_t *tx, sentry_span_t *span); + +/** + * Finishes the active transaction (if any) with `status`, closing out every + * in-flight child span in leaf-first order and shipping the tx envelope. + * `scope->span` / `scope->transaction_object` are preserved so a + * subsequently-captured crash event still inherits the active trace context. + * No-op if nothing is active. */ void sentry__trace_finish(sentry_span_status_t status); diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index be6fc9c50..1d4fea2d3 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -912,7 +912,30 @@ def test_trace_finish_on_crash(cmake, httpserver, backend): tx = tx_items[0].payload.json assert tx["contexts"]["trace"]["status"] == "aborted" - assert any(s.get("op") == "open.span" for s in tx.get("spans", [])) + spans = tx.get("spans", []) + # Every in-flight child in the active span chain should have been finished + # with aborted status, not just the deepest one. + for op in ("open.span", "open.grand.span"): + span = next((s for s in spans if s.get("op") == op), None) + assert span is not None, f"missing {op} in {[s.get('op') for s in spans]}" + assert span.get("status") == "aborted" + assert span.get("timestamp") + + # The crash event should nest under the deepest active span (open.grand.span) + # via matching trace_id + span_id, so Sentry renders it inside the chain + # rather than orphaning it at the trace root. + event_items = [ + item + for req, _ in httpserver.log + for item in Envelope.deserialize(req.get_data()).items + if item.headers.get("type") == "event" + ] + assert event_items + event = event_items[0].payload.json + grand = next(s for s in spans if s.get("op") == "open.grand.span") + assert event["contexts"]["trace"]["trace_id"] == tx["contexts"]["trace"]["trace_id"] + assert event["contexts"]["trace"]["span_id"] == grand["span_id"] + assert event.get("level") == "fatal" @pytest.mark.skipif(not has_files, reason="test needs a local filesystem") diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index 67b9d5dee..18bdcc071 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -790,6 +790,17 @@ check_aborted_transaction(sentry_envelope_t *envelope, void *data) sentry_value_get_by_key(tx, "contexts"), "trace"), "status", "aborted"); + // Every in-flight child span should be finished with aborted status and a + // timestamp so the crash event can nest under the active span chain. + sentry_value_t spans = sentry_value_get_by_key(tx, "spans"); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(spans), 2); + for (size_t i = 0; i < sentry_value_get_length(spans); i++) { + sentry_value_t span = sentry_value_get_by_index(spans, i); + CHECK_STRING_PROPERTY(span, "status", "aborted"); + TEST_CHECK( + !sentry_value_is_null(sentry_value_get_by_key(span, "timestamp"))); + } + sentry_envelope_free(envelope); } @@ -816,14 +827,19 @@ SENTRY_TEST(trace_finish) = sentry_transaction_start(ctx, sentry_value_new_null()); sentry_set_transaction_object(tx); - sentry_set_span(sentry_transaction_start_child(tx, "child-op", "child")); + sentry_span_t *child + = sentry_transaction_start_child(tx, "child-op", "child"); + sentry_set_span(sentry_span_start_child(child, "grand-op", "grand")); sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); - // Scope is torn down: no active span or tx remains. + // Scope keeps pointing at the (now finished) active span so a + // subsequently-captured crash event still inherits its trace context via + // `sentry__apply_scope_to_event`. The span carries `timestamp` now, but + // otherwise its trace_id / span_id / parent_span_id are still valid for + // scope apply. SENTRY_WITH_SCOPE (scope) { - TEST_CHECK(scope->span == NULL); - TEST_CHECK(scope->transaction_object == NULL); + TEST_CHECK(scope->span != NULL); } sentry_close(); From 0fa03bec265ac821cb7e2e4d0c57011d0b5d75e7 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 18:18:53 +0200 Subject: [PATCH 03/20] ref(tracing): Simplify trace-finish comments and surface alloc failure Trim WHAT-narration comments in sentry__trace_finish and its tests, keeping the sentry-cocoa alignment anchor and the non-obvious "detach to skip remove-scan" rationale. Warn on the tracking-list allocation failure so a silent crash-finalize gap is audible. Use the typedef name for the forward declaration in sentry_sampling_context.h for consistency with the rest of the codebase. Co-Authored-By: Claude Opus 4.7 (1M context) --- examples/example.c | 6 ++---- src/sentry_sampling_context.h | 2 +- src/sentry_tracing.c | 21 ++++++++------------- tests/test_integration_http.py | 8 +++----- tests/unit/test_tracing.c | 11 +++-------- 5 files changed, 17 insertions(+), 31 deletions(-) diff --git a/examples/example.c b/examples/example.c index 5a7571ffb..b665190e2 100644 --- a/examples/example.c +++ b/examples/example.c @@ -1050,10 +1050,8 @@ main(int argc, char **argv) } if (has_arg(argc, argv, "open-transaction")) { - // Start a transaction + nested child spans on the scope and - // intentionally do not finish them. On a subsequent crash, the - // backend's auto-finalize is expected to ship the transaction envelope - // with all the in-flight children closed out, not just the deepest. + // Leave a transaction + nested children unfinished; the crash + // auto-finalize should close them all. sentry_transaction_context_t *otx_ctx = sentry_transaction_context_new("open.tx", "op"); sentry_transaction_t *otx diff --git a/src/sentry_sampling_context.h b/src/sentry_sampling_context.h index 6977f52cb..3d7ec5253 100644 --- a/src/sentry_sampling_context.h +++ b/src/sentry_sampling_context.h @@ -6,7 +6,7 @@ #include "sentry_value.h" typedef struct sentry_sampling_context_s { - struct sentry_transaction_context_s *transaction_context; + sentry_transaction_context_t *transaction_context; sentry_value_t custom_sampling_context; bool *parent_sampled; double sample_rand; diff --git a/src/sentry_tracing.c b/src/sentry_tracing.c index a1ce80ebb..9e2016946 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -563,6 +563,8 @@ sentry__span_new(sentry_transaction_t *tx, sentry_value_t inner) } tx->children = new_children; tx->children_cap = new_cap; + } else { + SENTRY_WARN("failed to track live span for crash auto-finalize"); } } if (tx->children_count < tx->children_cap) { @@ -1022,12 +1024,10 @@ sentry_transaction_iter_headers(sentry_transaction_t *tx, void sentry__trace_finish(sentry_span_status_t status) { - // Save `scope->span` / `scope->transaction_object` so the subsequent crash - // event still sees the active trace context (matches sentry-cocoa's - // `finishTracer:shouldCleanUp:NO`). Finishing the tx through the normal - // path clears both; we restore them after the drain so the finished spans - // — which now carry `timestamp` but otherwise retain trace_id, span_id, - // parent_span_id, op, etc. — stay on scope for event capture. + // Save/restore scope around the drain so the crash event captured next + // still inherits the active trace context (cf. sentry-cocoa's + // `finishTracer:shouldCleanUp:NO`). Finished spans retain their ids; only + // `timestamp` is added. sentry_span_t *saved_span = NULL; sentry_transaction_t *saved_tx_obj = NULL; sentry_transaction_t *active_tx = NULL; @@ -1054,9 +1054,8 @@ sentry__trace_finish(sentry_span_status_t status) return; } - // Swap out the live-children list so `sentry_span_finish_ts`'s remove path - // becomes a no-op for the drained spans; iterate in reverse (leaf-first) - // so `scope->span` is cleared before its ancestors are finished. + // Detach the live-children list so each drained `sentry_span_finish_ts` + // skips the remove-scan. sentry__mutex_lock(&active_tx->children_mutex); sentry_span_t **drained = active_tx->children; size_t drained_count = active_tx->children_count; @@ -1077,10 +1076,6 @@ sentry__trace_finish(sentry_span_status_t status) sentry_transaction_set_status(active_tx, status); sentry_transaction_finish_ts(active_tx, end_ts); - // Restore scope so `sentry__apply_scope_to_event` (called when the crash - // event is captured right after) picks up the full trace context off the - // saved span/tx rather than falling through to the stale propagation - // context. SENTRY_WITH_SCOPE_MUT (scope) { if (!scope->span && saved_span) { scope->span = saved_span; diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 1d4fea2d3..0390b418b 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -913,17 +913,15 @@ def test_trace_finish_on_crash(cmake, httpserver, backend): tx = tx_items[0].payload.json assert tx["contexts"]["trace"]["status"] == "aborted" spans = tx.get("spans", []) - # Every in-flight child in the active span chain should have been finished - # with aborted status, not just the deepest one. + # Every in-flight child is finished, not just the deepest. for op in ("open.span", "open.grand.span"): span = next((s for s in spans if s.get("op") == op), None) assert span is not None, f"missing {op} in {[s.get('op') for s in spans]}" assert span.get("status") == "aborted" assert span.get("timestamp") - # The crash event should nest under the deepest active span (open.grand.span) - # via matching trace_id + span_id, so Sentry renders it inside the chain - # rather than orphaning it at the trace root. + # The crash event nests under the deepest active span via matching + # trace_id + span_id. event_items = [ item for req, _ in httpserver.log diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index 18bdcc071..1e7b4f0b9 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -790,8 +790,7 @@ check_aborted_transaction(sentry_envelope_t *envelope, void *data) sentry_value_get_by_key(tx, "contexts"), "trace"), "status", "aborted"); - // Every in-flight child span should be finished with aborted status and a - // timestamp so the crash event can nest under the active span chain. + // Every in-flight child is finished, not just the deepest. sentry_value_t spans = sentry_value_get_by_key(tx, "spans"); TEST_CHECK_INT_EQUAL(sentry_value_get_length(spans), 2); for (size_t i = 0; i < sentry_value_get_length(spans); i++) { @@ -833,18 +832,14 @@ SENTRY_TEST(trace_finish) sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); - // Scope keeps pointing at the (now finished) active span so a - // subsequently-captured crash event still inherits its trace context via - // `sentry__apply_scope_to_event`. The span carries `timestamp` now, but - // otherwise its trace_id / span_id / parent_span_id are still valid for - // scope apply. + // Scope still points at the (finished) span so a subsequent crash event + // inherits its trace context. SENTRY_WITH_SCOPE (scope) { TEST_CHECK(scope->span != NULL); } sentry_close(); - // The tx envelope was sent with status=aborted. TEST_CHECK_INT_EQUAL(called, 1); } From 2ddf45b331ddf74f533f3caf4c69425cce9e099e Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 18:58:32 +0200 Subject: [PATCH 04/20] ref(tracing): Extract save/restore trace and finish_children helpers Split sentry__trace_finish into save_active_trace / restore_active_trace (as a pair around the scope mutation) and finish_children for the atomic children swap + finish loop, so the top-level function reads as a short narrative. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/sentry_tracing.c | 120 +++++++++++++++++++++++++------------------ 1 file changed, 71 insertions(+), 49 deletions(-) diff --git a/src/sentry_tracing.c b/src/sentry_tracing.c index 9e2016946..96830062b 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -1021,71 +1021,93 @@ sentry_transaction_iter_headers(sentry_transaction_t *tx, } } -void -sentry__trace_finish(sentry_span_status_t status) +typedef struct { + sentry_span_t *saved_span; + sentry_transaction_t *saved_tx_obj; + sentry_transaction_t *active_tx; +} saved_trace_t; + +static saved_trace_t +save_active_trace(void) { - // Save/restore scope around the drain so the crash event captured next - // still inherits the active trace context (cf. sentry-cocoa's - // `finishTracer:shouldCleanUp:NO`). Finished spans retain their ids; only - // `timestamp` is added. - sentry_span_t *saved_span = NULL; - sentry_transaction_t *saved_tx_obj = NULL; - sentry_transaction_t *active_tx = NULL; + saved_trace_t s = { 0 }; SENTRY_WITH_SCOPE (scope) { if (scope->span) { sentry__span_incref(scope->span); - saved_span = scope->span; + s.saved_span = scope->span; } if (scope->transaction_object) { sentry__transaction_incref(scope->transaction_object); - saved_tx_obj = scope->transaction_object; - } - if (saved_span && saved_span->transaction) { - sentry__transaction_incref(saved_span->transaction); - active_tx = saved_span->transaction; - } else if (saved_tx_obj) { - sentry__transaction_incref(saved_tx_obj); - active_tx = saved_tx_obj; + s.saved_tx_obj = scope->transaction_object; } } - if (!active_tx) { - sentry__span_decref(saved_span); - sentry__transaction_decref(saved_tx_obj); - return; + s.active_tx = s.saved_span && s.saved_span->transaction + ? s.saved_span->transaction + : s.saved_tx_obj; + if (s.active_tx) { + sentry__transaction_incref(s.active_tx); } + return s; +} - // Detach the live-children list so each drained `sentry_span_finish_ts` - // skips the remove-scan. - sentry__mutex_lock(&active_tx->children_mutex); - sentry_span_t **drained = active_tx->children; - size_t drained_count = active_tx->children_count; - active_tx->children = NULL; - active_tx->children_count = 0; - active_tx->children_cap = 0; - sentry__mutex_unlock(&active_tx->children_mutex); +static void +restore_active_trace(saved_trace_t *s) +{ + SENTRY_WITH_SCOPE_MUT (scope) { + if (!scope->span && s->saved_span) { + scope->span = s->saved_span; + s->saved_span = NULL; + } + if (!scope->transaction_object && s->saved_tx_obj) { + scope->transaction_object = s->saved_tx_obj; + s->saved_tx_obj = NULL; + } + } + sentry__span_decref(s->saved_span); + sentry__transaction_decref(s->saved_tx_obj); + sentry__transaction_decref(s->active_tx); +} - uint64_t end_ts = sentry__usec_time(); - for (size_t i = drained_count; i-- > 0;) { - sentry_span_t *child = drained[i]; +// Atomically swap the live-children list off `tx` and finish each span. +// The swap ensures `sentry_span_finish_ts`'s per-span remove-scan is a no-op. +static void +finish_children( + sentry_transaction_t *tx, sentry_span_status_t status, uint64_t end_ts) +{ + sentry__mutex_lock(&tx->children_mutex); + sentry_span_t **children = tx->children; + size_t count = tx->children_count; + tx->children = NULL; + tx->children_count = 0; + tx->children_cap = 0; + sentry__mutex_unlock(&tx->children_mutex); + + for (size_t i = count; i-- > 0;) { + sentry_span_t *child = children[i]; sentry_span_set_status(child, status); sentry_span_finish_ts(child, end_ts); sentry__span_decref(child); } - sentry_free(drained); - - sentry_transaction_set_status(active_tx, status); - sentry_transaction_finish_ts(active_tx, end_ts); + sentry_free(children); +} - SENTRY_WITH_SCOPE_MUT (scope) { - if (!scope->span && saved_span) { - scope->span = saved_span; - saved_span = NULL; - } - if (!scope->transaction_object && saved_tx_obj) { - scope->transaction_object = saved_tx_obj; - saved_tx_obj = NULL; - } +void +sentry__trace_finish(sentry_span_status_t status) +{ + // Save/restore scope around the finish so the crash event captured next + // still inherits the active trace context (cf. sentry-cocoa's + // `finishTracer:shouldCleanUp:NO`). Finished spans retain their ids; only + // `timestamp` is added. + saved_trace_t s = save_active_trace(); + if (!s.active_tx) { + return; } - sentry__span_decref(saved_span); - sentry__transaction_decref(saved_tx_obj); + + uint64_t end_ts = sentry__usec_time(); + finish_children(s.active_tx, status, end_ts); + + sentry_transaction_set_status(s.active_tx, status); + sentry_transaction_finish_ts(s.active_tx, end_ts); + + restore_active_trace(&s); } From ec5eeeae6c38be661a001e93c27a242d78a97e57 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 19:19:40 +0200 Subject: [PATCH 05/20] fix(tracing): Don't double-decref active_tx in restore_active_trace sentry_transaction_finish_ts consumes its argument, so the ref that save_active_trace took for active_tx is released by the finish call. restore_active_trace was decref'ing it again. Harmless in crash context where the process exits, but a real refcount underflow otherwise. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/sentry_tracing.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sentry_tracing.c b/src/sentry_tracing.c index 96830062b..3da5446e1 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -1065,7 +1065,6 @@ restore_active_trace(saved_trace_t *s) } sentry__span_decref(s->saved_span); sentry__transaction_decref(s->saved_tx_obj); - sentry__transaction_decref(s->active_tx); } // Atomically swap the live-children list off `tx` and finish each span. From 8612326368f419d1d11c19ff7a661f8bddc04b30 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 19:28:03 +0200 Subject: [PATCH 06/20] fix(tracing): Don't double-decref children in finish_children sentry_span_finish_ts consumes its argument (decref at sentry_core.c:1607 on success, :1611 on fail), so the explicit sentry__span_decref after the finish call released the children-list ref a second time. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/sentry_tracing.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/sentry_tracing.c b/src/sentry_tracing.c index 3da5446e1..0d4964479 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -1085,7 +1085,6 @@ finish_children( sentry_span_t *child = children[i]; sentry_span_set_status(child, status); sentry_span_finish_ts(child, end_ts); - sentry__span_decref(child); } sentry_free(children); } From 62cc38626b8225072ef65880f91c5fc47cd20fa1 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 19:55:15 +0200 Subject: [PATCH 07/20] fix(tracing): Break tx<->span refcount cycle with weak children list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The transaction's `children` list held an incref'd reference on each live span, while each span holds a ref on its transaction via `span->transaction`. Any code path that decref'd a span without going through `sentry_span_finish_ts` (e.g. the `span_data` unit test using the low-level `sentry__span_new` / `sentry__span_decref` API) left both sides stuck at refcount 1, leaking both. Make `tx->children` weak: `sentry__span_new` no longer increfs on add, and `sentry__span_decref` unlists the span from the children list on its final drop. `sentry__transaction_remove_child` correspondingly no longer decrefs. The drain path in `sentry__trace_finish` continues to work — the spans on the swapped-out list are alive via their other refs (scope / saved_span / user var), and `sentry_span_finish_ts` consumes one of those refs as the caller's, exactly as in the non-crash flow. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/sentry_tracing.c | 10 +--------- src/sentry_tracing.h | 8 ++++---- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/src/sentry_tracing.c b/src/sentry_tracing.c index 0d4964479..b9253f56b 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -477,9 +477,6 @@ sentry__transaction_decref(sentry_transaction_t *tx) if (sentry_value_refcount(tx->inner) <= 1) { sentry_value_decref(tx->inner); - for (size_t i = 0; i < tx->children_count; i++) { - sentry__span_decref(tx->children[i]); - } sentry_free(tx->children); sentry__mutex_free(&tx->children_mutex); sentry_free(tx); @@ -494,19 +491,14 @@ sentry__transaction_remove_child(sentry_transaction_t *tx, sentry_span_t *span) if (!tx || !span) { return; } - bool found = false; sentry__mutex_lock(&tx->children_mutex); for (size_t i = 0; i < tx->children_count; i++) { if (tx->children[i] == span) { tx->children[i] = tx->children[--tx->children_count]; - found = true; break; } } sentry__mutex_unlock(&tx->children_mutex); - if (found) { - sentry__span_decref(span); - } } void @@ -525,6 +517,7 @@ sentry__span_decref(sentry_span_t *span) } if (sentry_value_refcount(span->inner) <= 1) { + sentry__transaction_remove_child(span->transaction, span); sentry_value_decref(span->inner); sentry__transaction_decref(span->transaction); sentry_free(span); @@ -568,7 +561,6 @@ sentry__span_new(sentry_transaction_t *tx, sentry_value_t inner) } } if (tx->children_count < tx->children_cap) { - sentry__span_incref(span); tx->children[tx->children_count++] = span; } sentry__mutex_unlock(&tx->children_mutex); diff --git a/src/sentry_tracing.h b/src/sentry_tracing.h index 0d9599c9e..990e86e72 100644 --- a/src/sentry_tracing.h +++ b/src/sentry_tracing.h @@ -35,8 +35,8 @@ struct sentry_transaction_context_s { struct sentry_transaction_s { sentry_value_t inner; // Live (unfinished) child spans, so `sentry__trace_finish` can close them - // out on crash. Entries are added in `sentry__span_new` and removed in - // `sentry_span_finish_ts`. Each entry holds one ref on the span. + // out on crash. Weak pointers: entries do not own a ref — spans remove + // themselves via `sentry__transaction_remove_child` on finish or decref. sentry_mutex_t children_mutex; sentry_span_t **children; size_t children_count; @@ -50,8 +50,8 @@ void sentry__transaction_incref(sentry_transaction_t *tx); void sentry__transaction_decref(sentry_transaction_t *tx); /** - * Removes `span` from the transaction's live-children list and drops the ref - * that was taken by `sentry__span_new`. No-op if not found. + * Unlists `span` from the transaction's live-children list. No-op if not + * found. Does not decref (the list holds weak pointers). */ void sentry__transaction_remove_child( sentry_transaction_t *tx, sentry_span_t *span); From abcead6b7bb80fc0e0905078142d6ecc7ad7e3b8 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 21:22:33 +0200 Subject: [PATCH 08/20] fix(tracing): Incref each span in finish_children drain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the weak-children refactor, `finish_children` handed each span to `sentry_span_finish_ts` without a caller ref. `finish_ts` consumes one ref on its argument, so when the drained span had only the user's own ref outstanding, finish_ts would drop it to zero and free the span — leaving the user's pointer dangling. Incref inside the drain loop so finish_ts consumes "our" ref, leaving user refs intact. Also update the `trace_finish` unit test to release the refs it held on `tx`/`child`/`grand` (without the children-list strong ref, unfinished spans now properly leak if the user forgets to decref). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/sentry_tracing.c | 1 + tests/unit/test_tracing.c | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/sentry_tracing.c b/src/sentry_tracing.c index b9253f56b..5b4708da5 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -1075,6 +1075,7 @@ finish_children( for (size_t i = count; i-- > 0;) { sentry_span_t *child = children[i]; + sentry__span_incref(child); sentry_span_set_status(child, status); sentry_span_finish_ts(child, end_ts); } diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index 1e7b4f0b9..3ef3a513a 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -828,7 +828,8 @@ SENTRY_TEST(trace_finish) sentry_span_t *child = sentry_transaction_start_child(tx, "child-op", "child"); - sentry_set_span(sentry_span_start_child(child, "grand-op", "grand")); + sentry_span_t *grand = sentry_span_start_child(child, "grand-op", "grand"); + sentry_set_span(grand); sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); @@ -838,6 +839,10 @@ SENTRY_TEST(trace_finish) TEST_CHECK(scope->span != NULL); } + sentry__span_decref(grand); + sentry__span_decref(child); + sentry__transaction_decref(tx); + sentry_close(); TEST_CHECK_INT_EQUAL(called, 1); From 28791c4af1d8240bce9e384562cebb1c0387675d Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 24 Apr 2026 06:57:15 +0200 Subject: [PATCH 09/20] fix(tracing): Use NO_FLUSH scope macro in restore_active_trace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SENTRY_WITH_SCOPE_MUT calls sentry__scope_flush_unlock on exit, which invokes the backend's flush_scope_func — file I/O in both crashpad and native backends. sentry__trace_finish runs from signal-handler context in the inproc/breakpad/native crash handlers, so the flush is unsafe there. Use the NO_FLUSH variant the codebase provides for this exact situation. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/sentry_tracing.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry_tracing.c b/src/sentry_tracing.c index 5b4708da5..dfad11c82 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -1045,7 +1045,7 @@ save_active_trace(void) static void restore_active_trace(saved_trace_t *s) { - SENTRY_WITH_SCOPE_MUT (scope) { + SENTRY_WITH_SCOPE_MUT_NO_FLUSH (scope) { if (!scope->span && s->saved_span) { scope->span = s->saved_span; s->saved_span = NULL; From d4596758befe654bf41f43207bfa452e384973ea Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 24 Apr 2026 07:23:18 +0200 Subject: [PATCH 10/20] fix(tracing): Restore cleanup on sentry__trace_finish early return Call restore_active_trace before the early return so any refs taken by save_active_trace are released even if active_tx is NULL. No-op under the current invariant (saved_span->transaction is always non-NULL), but defensive if the invariant ever changes. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/sentry_tracing.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sentry_tracing.c b/src/sentry_tracing.c index dfad11c82..da5428c05 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -1091,6 +1091,7 @@ sentry__trace_finish(sentry_span_status_t status) // `timestamp` is added. saved_trace_t s = save_active_trace(); if (!s.active_tx) { + restore_active_trace(&s); return; } From 257d2de4a4edbd7e5685f7ac5e3e5bb4ebd75557 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 24 Apr 2026 11:36:07 +0200 Subject: [PATCH 11/20] Revert "fix(tracing): Use NO_FLUSH scope macro in restore_active_trace" This reverts commit b0c1aca59d7c71c32f7e7e0d14ff9532ff02b880. --- src/sentry_tracing.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sentry_tracing.c b/src/sentry_tracing.c index da5428c05..8a0f6ce4f 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -1045,7 +1045,7 @@ save_active_trace(void) static void restore_active_trace(saved_trace_t *s) { - SENTRY_WITH_SCOPE_MUT_NO_FLUSH (scope) { + SENTRY_WITH_SCOPE_MUT (scope) { if (!scope->span && s->saved_span) { scope->span = s->saved_span; s->saved_span = NULL; From ea9dce5372430faf9dff71eef074f78cf27213be Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 27 Apr 2026 12:38:48 +0200 Subject: [PATCH 12/20] test: isolate native trace crash uploads --- tests/test_integration_http.py | 113 +++++++++++++++++++++------------ 1 file changed, 72 insertions(+), 41 deletions(-) diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 0390b418b..9e5269cbf 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -854,6 +854,41 @@ def test_native_crash_http(cmake, httpserver): assert_attachment(envelope) +def assert_trace_finish_on_crash(httpserver): + tx_items = [ + item + for req, _ in httpserver.log + for item in Envelope.deserialize(req.get_data()).items + if item.headers.get("type") == "transaction" + ] + assert tx_items + + tx = tx_items[0].payload.json + assert tx["contexts"]["trace"]["status"] == "aborted" + spans = tx.get("spans", []) + # Every in-flight child is finished, not just the deepest. + for op in ("open.span", "open.grand.span"): + span = next((s for s in spans if s.get("op") == op), None) + assert span is not None, f"missing {op} in {[s.get('op') for s in spans]}" + assert span.get("status") == "aborted" + assert span.get("timestamp") + + # The crash event nests under the deepest active span via matching + # trace_id + span_id. + event_items = [ + item + for req, _ in httpserver.log + for item in Envelope.deserialize(req.get_data()).items + if item.headers.get("type") == "event" + ] + assert event_items + event = event_items[0].payload.json + grand = next(s for s in spans if s.get("op") == "open.grand.span") + assert event["contexts"]["trace"]["trace_id"] == tx["contexts"]["trace"]["trace_id"] + assert event["contexts"]["trace"]["span_id"] == grand["span_id"] + assert event.get("level") == "fatal" + + @pytest.mark.parametrize( "backend", [ @@ -864,13 +899,6 @@ def test_native_crash_http(cmake, httpserver): not has_breakpad or is_qemu, reason="test needs breakpad backend" ), ), - pytest.param( - "native", - marks=pytest.mark.skipif( - not has_native or is_qemu or is_kcov, - reason="test needs native backend", - ), - ), ], ) def test_trace_finish_on_crash(cmake, httpserver, backend): @@ -878,7 +906,7 @@ def test_trace_finish_on_crash(cmake, httpserver, backend): unfinished transaction on the scope ships alongside the crash.""" tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": backend}) - # Expect the crash envelope + the auto-finalized tx envelope. + # crash + auto-finished transaction httpserver.expect_oneshot_request( "/api/123456/envelope/", headers={"x-sentry-auth": auth_header}, @@ -897,43 +925,46 @@ def test_trace_finish_on_crash(cmake, httpserver, backend): expect_failure=True, env=env, ) - if backend != "native": - # inproc/breakpad cache to disk; the next launch ships them. - run(tmp_path, "sentry_example", ["log", "no-setup"], env=env) + # inproc/breakpad cache to disk; the next launch ships them. + run(tmp_path, "sentry_example", ["log", "no-setup"], env=env) assert waiting.result - tx_items = [ - item - for req, _ in httpserver.log - for item in Envelope.deserialize(req.get_data()).items - if item.headers.get("type") == "transaction" - ] - assert tx_items + assert_trace_finish_on_crash(httpserver) - tx = tx_items[0].payload.json - assert tx["contexts"]["trace"]["status"] == "aborted" - spans = tx.get("spans", []) - # Every in-flight child is finished, not just the deepest. - for op in ("open.span", "open.grand.span"): - span = next((s for s in spans if s.get("op") == op), None) - assert span is not None, f"missing {op} in {[s.get('op') for s in spans]}" - assert span.get("status") == "aborted" - assert span.get("timestamp") - # The crash event nests under the deepest active span via matching - # trace_id + span_id. - event_items = [ - item - for req, _ in httpserver.log - for item in Envelope.deserialize(req.get_data()).items - if item.headers.get("type") == "event" - ] - assert event_items - event = event_items[0].payload.json - grand = next(s for s in spans if s.get("op") == "open.grand.span") - assert event["contexts"]["trace"]["trace_id"] == tx["contexts"]["trace"]["trace_id"] - assert event["contexts"]["trace"]["span_id"] == grand["span_id"] - assert event.get("level") == "fatal" +@pytest.mark.skipif( + not has_native or is_qemu or is_kcov, reason="test needs native backend" +) +def test_trace_finish_on_crash_native(cmake, httpserver): + tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "native"}) + + # crash + auto-finished transaction + httpserver.expect_oneshot_request( + "/api/123456/envelope/", + headers={"x-sentry-auth": auth_header}, + ).respond_with_data("OK") + httpserver.expect_oneshot_request( + "/api/123456/envelope/", + headers={"x-sentry-auth": auth_header}, + ).respond_with_data("OK") + # TODO: fix duplicate tx + httpserver.expect_oneshot_request( + "/api/123456/envelope/", + headers={"x-sentry-auth": auth_header}, + ).respond_with_data("OK") + env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) + + with httpserver.wait(timeout=10) as waiting: + run( + tmp_path, + "sentry_example", + ["log", "open-transaction", "crash"], + expect_failure=True, + env=env, + ) + assert waiting.result + + assert_trace_finish_on_crash(httpserver) @pytest.mark.skipif(not has_files, reason="test needs a local filesystem") From 810bc780f5cbd0977aa34fa1b02204212004d1c4 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 27 Apr 2026 13:39:03 +0200 Subject: [PATCH 13/20] fix: avoid live transport for crash trace finish --- src/backends/native/sentry_crash_daemon.c | 58 +++++++++++++++++++++ src/backends/sentry_backend_breakpad.cpp | 13 ++++- src/backends/sentry_backend_inproc.c | 15 ++++-- src/backends/sentry_backend_native.c | 30 ++++++++++- src/sentry_core.c | 63 ++++++++++++++++------- src/sentry_core.h | 6 +++ src/sentry_envelope.c | 5 +- src/sentry_tracing.c | 7 +-- src/sentry_tracing.h | 6 +-- tests/test_integration_http.py | 15 ------ tests/unit/test_tracing.c | 43 +++++++--------- 11 files changed, 191 insertions(+), 70 deletions(-) diff --git a/src/backends/native/sentry_crash_daemon.c b/src/backends/native/sentry_crash_daemon.c index b440aa1d2..616ce1826 100644 --- a/src/backends/native/sentry_crash_daemon.c +++ b/src/backends/native/sentry_crash_daemon.c @@ -303,6 +303,44 @@ add_attachment_refs(sentry_envelope_t *envelope, sentry_value_decref(list); } +static bool +write_json_item_to_envelope(int fd, const char *file_path, const char *type) +{ + sentry_path_t *path = sentry__path_from_str(file_path); + size_t size = 0; + char *json = path ? sentry__path_read_to_buffer(path, &size) : NULL; + sentry__path_free(path); + if (!json || size == 0) { + sentry_free(json); + return false; + } + + char header[SENTRY_CRASH_ITEM_HEADER_SIZE]; + int header_len = snprintf( + header, sizeof(header), "{\"type\":\"%s\",\"length\":%zu}\n", type, size); + if (header_len <= 0 || header_len >= (int)sizeof(header)) { + sentry_free(json); + return false; + } + +#if defined(SENTRY_PLATFORM_UNIX) + if (write(fd, header, header_len) != (ssize_t)header_len + || write(fd, json, size) != (ssize_t)size + || write(fd, "\n", 1) != 1) { + SENTRY_WARNF("Failed to write %s item to envelope", type); + sentry_free(json); + return false; + } +#elif defined(SENTRY_PLATFORM_WINDOWS) + _write(fd, header, (unsigned int)header_len); + _write(fd, json, (unsigned int)size); + _write(fd, "\n", 1); +#endif + + sentry_free(json); + return true; +} + #if defined(SENTRY_PLATFORM_UNIX) /** * Get signal name from signal number (Unix platforms only) @@ -2386,6 +2424,16 @@ write_envelope_with_native_stacktrace(const sentry_options_t *options, sentry_free(event_json); + if (run_folder) { + sentry_path_t *transaction_path + = sentry__path_join_str(run_folder, "__sentry-transaction"); + if (transaction_path) { + write_json_item_to_envelope( + fd, transaction_path->path, "transaction"); + sentry__path_free(transaction_path); + } + } + // Add minidump as attachment if provided if (minidump_path && minidump_path[0]) { #if defined(SENTRY_PLATFORM_UNIX) @@ -2639,6 +2687,16 @@ write_envelope_with_minidump(const sentry_options_t *options, } sentry_free(event_json); + if (run_folder) { + sentry_path_t *transaction_path + = sentry__path_join_str(run_folder, "__sentry-transaction"); + if (transaction_path) { + write_json_item_to_envelope( + fd, transaction_path->path, "transaction"); + sentry__path_free(transaction_path); + } + } + // Add minidump as attachment #if defined(SENTRY_PLATFORM_UNIX) int minidump_fd = open(minidump_path, O_RDONLY); diff --git a/src/backends/sentry_backend_breakpad.cpp b/src/backends/sentry_backend_breakpad.cpp index 49f4205b5..76e5fa770 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -140,7 +140,8 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, SENTRY_WITH_OPTIONS (options) { sentry__write_crash_marker(options); - sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); + sentry_value_t transaction + = sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); bool should_handle = true; @@ -188,6 +189,15 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, sentry_envelope_t *envelope = sentry__prepare_event( options, event, nullptr, !options->on_crash_func, nullptr); + if (!sentry_value_is_null(transaction)) { + transaction = sentry__prepare_transaction_value( + options, transaction, nullptr); + if (!sentry_value_is_null(transaction) + && !sentry__envelope_add_transaction( + envelope, transaction)) { + sentry_value_decref(transaction); + } + } sentry_session_t *session = sentry__end_current_session_with_status( SENTRY_SESSION_STATUS_CRASHED); sentry__envelope_add_session(envelope, session); @@ -259,6 +269,7 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, SENTRY_SIGNAL_SAFE_LOG( "DEBUG event was discarded by the `on_crash` hook"); sentry_value_decref(event); + sentry_value_decref(transaction); } // after capturing the crash event, try to dump all the in-flight diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 0a7585e0c..3c0934ef0 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -1116,7 +1116,8 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx, bool should_handle = true; sentry__write_crash_marker(options); - sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); + sentry_value_t transaction + = sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); if (options->on_crash_func && !skip_hooks) { SENTRY_DEBUG("invoking `on_crash` hook"); @@ -1147,8 +1148,15 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx, sentry_envelope_t *envelope = sentry__prepare_event(options, event, NULL, !options->on_crash_func && !skip_hooks, NULL); - // TODO(tracing): Revisit when investigating transaction flushing - // during hard crashes. + if (!sentry_value_is_null(transaction)) { + transaction + = sentry__prepare_transaction_value(options, transaction, NULL); + if (!sentry_value_is_null(transaction) + && !sentry__envelope_add_transaction( + envelope, transaction)) { + sentry_value_decref(transaction); + } + } sentry_session_t *session = sentry__end_current_session_with_status( SENTRY_SESSION_STATUS_CRASHED); @@ -1186,6 +1194,7 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx, } else { SENTRY_DEBUG("event was discarded by the `on_crash` hook"); sentry_value_decref(event); + sentry_value_decref(transaction); } // after capturing the crash event, dump all the envelopes to disk diff --git a/src/backends/sentry_backend_native.c b/src/backends/sentry_backend_native.c index 49e634fe4..62e70ae34 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -989,7 +989,8 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx) // Write crash marker sentry__write_crash_marker(options); - sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); + sentry_value_t transaction + = sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); // Create crash event sentry_value_t event = sentry_value_new_event(); @@ -1086,6 +1087,32 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx) } } + if (!sentry_value_is_null(transaction) && state + && state->event_path) { + transaction = sentry__prepare_transaction_value( + options, transaction, NULL); + if (!sentry_value_is_null(transaction)) { + char *transaction_json + = sentry_value_to_json(transaction); + sentry_path_t *run_path + = sentry__path_dir(state->event_path); + sentry_path_t *transaction_path = run_path + ? sentry__path_join_str( + run_path, "__sentry-transaction") + : NULL; + if (transaction_json && transaction_path) { + sentry__path_write_buffer(transaction_path, + transaction_json, strlen(transaction_json)); + } + sentry__path_free(transaction_path); + sentry__path_free(run_path); + sentry_free(transaction_json); + sentry_value_decref(transaction); + } + } else { + sentry_value_decref(transaction); + } + sentry_value_decref(event); // End session with crashed status and write session envelope to @@ -1127,6 +1154,7 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx) } else { SENTRY_DEBUG("event was discarded by the `on_crash` hook"); sentry_value_decref(event); + sentry_value_decref(transaction); } } } diff --git a/src/sentry_core.c b/src/sentry_core.c index 140084d0a..ac6fcd088 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -725,6 +725,32 @@ sentry__prepare_transaction(const sentry_options_t *options, { sentry_envelope_t *envelope = NULL; + transaction + = sentry__prepare_transaction_value(options, transaction, event_id); + if (sentry_value_is_null(transaction)) { + return NULL; + } + + envelope = sentry__envelope_new(); + if (!envelope || !sentry__envelope_add_transaction(envelope, transaction)) { + goto fail; + } + + // TODO(tracing): Revisit when adding attachment support for transactions. + + return envelope; + +fail: + SENTRY_WARN("dropping transaction"); + sentry_envelope_free(envelope); + sentry_value_decref(transaction); + return NULL; +} + +sentry_value_t +sentry__prepare_transaction_value(const sentry_options_t *options, + sentry_value_t transaction, sentry_uuid_t *event_id) +{ SENTRY_WITH_SCOPE (scope) { SENTRY_DEBUG("merging scope into transaction"); // Don't include debugging info @@ -742,25 +768,12 @@ sentry__prepare_transaction(const sentry_options_t *options, "transaction was discarded by the `before_transaction` hook"); sentry__client_report_discard(SENTRY_DISCARD_REASON_BEFORE_SEND, SENTRY_DATA_CATEGORY_TRANSACTION, 1); - return NULL; + return sentry_value_new_null(); } } sentry__ensure_event_id(transaction, event_id); - envelope = sentry__envelope_new(); - if (!envelope || !sentry__envelope_add_transaction(envelope, transaction)) { - goto fail; - } - - // TODO(tracing): Revisit when adding attachment support for transactions. - - return envelope; - -fail: - SENTRY_WARN("dropping transaction"); - sentry_envelope_free(envelope); - sentry_value_decref(transaction); - return NULL; + return transaction; } static sentry_envelope_t * @@ -1329,6 +1342,20 @@ sentry_transaction_finish(sentry_transaction_t *opaque_tx) sentry_uuid_t sentry_transaction_finish_ts( sentry_transaction_t *opaque_tx, uint64_t timestamp) +{ + sentry_value_t tx = sentry__transaction_finish_value(opaque_tx, timestamp); + if (sentry_value_is_null(tx)) { + return sentry_uuid_nil(); + } + + // This takes ownership of the transaction, generates an event ID, merges + // scope + return sentry__capture_event(tx, NULL); +} + +sentry_value_t +sentry__transaction_finish_value( + sentry_transaction_t *opaque_tx, uint64_t timestamp) { if (!opaque_tx || sentry_value_is_null(opaque_tx->inner)) { SENTRY_WARN("no transaction available to finish"); @@ -1409,12 +1436,10 @@ sentry_transaction_finish_ts( sentry__transaction_decref(opaque_tx); - // This takes ownership of the transaction, generates an event ID, merges - // scope - return sentry__capture_event(tx, NULL); + return tx; fail: sentry__transaction_decref(opaque_tx); - return sentry_uuid_nil(); + return sentry_value_new_null(); } void diff --git a/src/sentry_core.h b/src/sentry_core.h index c0b6f60fb..352303371 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -88,6 +88,12 @@ sentry_uuid_t sentry__capture_event( */ sentry_envelope_t *sentry__prepare_transaction(const sentry_options_t *options, sentry_value_t transaction, sentry_uuid_t *event_id); +sentry_value_t sentry__prepare_transaction_value( + const sentry_options_t *options, sentry_value_t transaction, + sentry_uuid_t *event_id); + +sentry_value_t sentry__transaction_finish_value( + sentry_transaction_t *opaque_tx, uint64_t timestamp); /** * This function will submit the `envelope` to the given `transport`, first diff --git a/src/sentry_envelope.c b/src/sentry_envelope.c index 655904302..ada0ea7c0 100644 --- a/src/sentry_envelope.c +++ b/src/sentry_envelope.c @@ -380,7 +380,10 @@ sentry__envelope_add_event(sentry_envelope_t *envelope, sentry_value_t event) sentry_value_t length = sentry_value_new_int32((int32_t)item->payload_len); sentry__envelope_item_set_header(item, "length", length); - sentry__envelope_set_event_id(envelope, &event_id); + sentry_uuid_t envelope_event_id = sentry__envelope_get_event_id(envelope); + if (sentry_uuid_is_nil(&envelope_event_id)) { + sentry__envelope_set_event_id(envelope, &event_id); + } double traces_sample_rate = 0.0; SENTRY_WITH_OPTIONS (options) { diff --git a/src/sentry_tracing.c b/src/sentry_tracing.c index 8a0f6ce4f..8c32a97c9 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -1082,7 +1082,7 @@ finish_children( sentry_free(children); } -void +sentry_value_t sentry__trace_finish(sentry_span_status_t status) { // Save/restore scope around the finish so the crash event captured next @@ -1092,14 +1092,15 @@ sentry__trace_finish(sentry_span_status_t status) saved_trace_t s = save_active_trace(); if (!s.active_tx) { restore_active_trace(&s); - return; + return sentry_value_new_null(); } uint64_t end_ts = sentry__usec_time(); finish_children(s.active_tx, status, end_ts); sentry_transaction_set_status(s.active_tx, status); - sentry_transaction_finish_ts(s.active_tx, end_ts); + sentry_value_t tx = sentry__transaction_finish_value(s.active_tx, end_ts); restore_active_trace(&s); + return tx; } diff --git a/src/sentry_tracing.h b/src/sentry_tracing.h index 990e86e72..5ba423d17 100644 --- a/src/sentry_tracing.h +++ b/src/sentry_tracing.h @@ -58,12 +58,12 @@ void sentry__transaction_remove_child( /** * Finishes the active transaction (if any) with `status`, closing out every - * in-flight child span in leaf-first order and shipping the tx envelope. + * in-flight child span in leaf-first order and returning the tx payload. * `scope->span` / `scope->transaction_object` are preserved so a * subsequently-captured crash event still inherits the active trace context. - * No-op if nothing is active. + * Returns null if nothing is active. */ -void sentry__trace_finish(sentry_span_status_t status); +sentry_value_t sentry__trace_finish(sentry_span_status_t status); void sentry__span_incref(sentry_span_t *span); void sentry__span_decref(sentry_span_t *span); diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 9e5269cbf..5a421cd83 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -906,11 +906,6 @@ def test_trace_finish_on_crash(cmake, httpserver, backend): unfinished transaction on the scope ships alongside the crash.""" tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": backend}) - # crash + auto-finished transaction - httpserver.expect_oneshot_request( - "/api/123456/envelope/", - headers={"x-sentry-auth": auth_header}, - ).respond_with_data("OK") httpserver.expect_oneshot_request( "/api/123456/envelope/", headers={"x-sentry-auth": auth_header}, @@ -938,16 +933,6 @@ def test_trace_finish_on_crash(cmake, httpserver, backend): def test_trace_finish_on_crash_native(cmake, httpserver): tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "native"}) - # crash + auto-finished transaction - httpserver.expect_oneshot_request( - "/api/123456/envelope/", - headers={"x-sentry-auth": auth_header}, - ).respond_with_data("OK") - httpserver.expect_oneshot_request( - "/api/123456/envelope/", - headers={"x-sentry-auth": auth_header}, - ).respond_with_data("OK") - # TODO: fix duplicate tx httpserver.expect_oneshot_request( "/api/123456/envelope/", headers={"x-sentry-auth": auth_header}, diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index 3ef3a513a..9c6047fbf 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -779,42 +779,24 @@ check_spans(sentry_envelope_t *envelope, void *data) } static void -check_aborted_transaction(sentry_envelope_t *envelope, void *data) +count_envelope(sentry_envelope_t *envelope, void *data) { uint64_t *called = data; *called += 1; - - sentry_value_t tx = sentry_envelope_get_transaction(envelope); - TEST_CHECK(!sentry_value_is_null(tx)); - CHECK_STRING_PROPERTY(sentry_value_get_by_key( - sentry_value_get_by_key(tx, "contexts"), "trace"), - "status", "aborted"); - - // Every in-flight child is finished, not just the deepest. - sentry_value_t spans = sentry_value_get_by_key(tx, "spans"); - TEST_CHECK_INT_EQUAL(sentry_value_get_length(spans), 2); - for (size_t i = 0; i < sentry_value_get_length(spans); i++) { - sentry_value_t span = sentry_value_get_by_index(spans, i); - CHECK_STRING_PROPERTY(span, "status", "aborted"); - TEST_CHECK( - !sentry_value_is_null(sentry_value_get_by_key(span, "timestamp"))); - } - sentry_envelope_free(envelope); } SENTRY_TEST(trace_finish) { // No active span/tx: no-op, no crash. - sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); + sentry_value_decref(sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED)); uint64_t called = 0; SENTRY_TEST_OPTIONS_NEW(options); sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); sentry_options_set_auto_session_tracking(options, 0); - sentry_transport_t *transport - = sentry_transport_new(check_aborted_transaction); + sentry_transport_t *transport = sentry_transport_new(count_envelope); sentry_transport_set_state(transport, &called); sentry_options_set_transport(options, transport); sentry_options_set_traces_sample_rate(options, 1.0); @@ -831,7 +813,21 @@ SENTRY_TEST(trace_finish) sentry_span_t *grand = sentry_span_start_child(child, "grand-op", "grand"); sentry_set_span(grand); - sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); + sentry_value_t finished = sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); + TEST_CHECK(!sentry_value_is_null(finished)); + CHECK_STRING_PROPERTY(sentry_value_get_by_key( + sentry_value_get_by_key(finished, "contexts"), "trace"), + "status", "aborted"); + + sentry_value_t spans = sentry_value_get_by_key(finished, "spans"); + TEST_CHECK_INT_EQUAL(sentry_value_get_length(spans), 2); + for (size_t i = 0; i < sentry_value_get_length(spans); i++) { + sentry_value_t span = sentry_value_get_by_index(spans, i); + CHECK_STRING_PROPERTY(span, "status", "aborted"); + TEST_CHECK( + !sentry_value_is_null(sentry_value_get_by_key(span, "timestamp"))); + } + sentry_value_decref(finished); // Scope still points at the (finished) span so a subsequent crash event // inherits its trace context. @@ -844,8 +840,7 @@ SENTRY_TEST(trace_finish) sentry__transaction_decref(tx); sentry_close(); - - TEST_CHECK_INT_EQUAL(called, 1); + TEST_CHECK_INT_EQUAL(called, 0); } SENTRY_TEST(drop_unfinished_spans) From ab07daf36371eb0d08c090da3362fcf99cc1f91c Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 27 Apr 2026 13:53:55 +0200 Subject: [PATCH 14/20] Revert test changes --- tests/test_integration_http.py | 102 +++++++++++++-------------------- 1 file changed, 40 insertions(+), 62 deletions(-) diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 5a421cd83..444640fa3 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -853,42 +853,6 @@ def test_native_crash_http(cmake, httpserver): assert_breadcrumb(envelope) assert_attachment(envelope) - -def assert_trace_finish_on_crash(httpserver): - tx_items = [ - item - for req, _ in httpserver.log - for item in Envelope.deserialize(req.get_data()).items - if item.headers.get("type") == "transaction" - ] - assert tx_items - - tx = tx_items[0].payload.json - assert tx["contexts"]["trace"]["status"] == "aborted" - spans = tx.get("spans", []) - # Every in-flight child is finished, not just the deepest. - for op in ("open.span", "open.grand.span"): - span = next((s for s in spans if s.get("op") == op), None) - assert span is not None, f"missing {op} in {[s.get('op') for s in spans]}" - assert span.get("status") == "aborted" - assert span.get("timestamp") - - # The crash event nests under the deepest active span via matching - # trace_id + span_id. - event_items = [ - item - for req, _ in httpserver.log - for item in Envelope.deserialize(req.get_data()).items - if item.headers.get("type") == "event" - ] - assert event_items - event = event_items[0].payload.json - grand = next(s for s in spans if s.get("op") == "open.grand.span") - assert event["contexts"]["trace"]["trace_id"] == tx["contexts"]["trace"]["trace_id"] - assert event["contexts"]["trace"]["span_id"] == grand["span_id"] - assert event.get("level") == "fatal" - - @pytest.mark.parametrize( "backend", [ @@ -899,6 +863,13 @@ def assert_trace_finish_on_crash(httpserver): not has_breakpad or is_qemu, reason="test needs breakpad backend" ), ), + pytest.param( + "native", + marks=pytest.mark.skipif( + not has_native or is_qemu or is_kcov, + reason="test needs native backend", + ), + ), ], ) def test_trace_finish_on_crash(cmake, httpserver, backend): @@ -920,36 +891,43 @@ def test_trace_finish_on_crash(cmake, httpserver, backend): expect_failure=True, env=env, ) - # inproc/breakpad cache to disk; the next launch ships them. - run(tmp_path, "sentry_example", ["log", "no-setup"], env=env) + if backend != "native": + # inproc/breakpad cache to disk; the next launch ships them. + run(tmp_path, "sentry_example", ["log", "no-setup"], env=env) assert waiting.result - assert_trace_finish_on_crash(httpserver) - - -@pytest.mark.skipif( - not has_native or is_qemu or is_kcov, reason="test needs native backend" -) -def test_trace_finish_on_crash_native(cmake, httpserver): - tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": "native"}) - - httpserver.expect_oneshot_request( - "/api/123456/envelope/", - headers={"x-sentry-auth": auth_header}, - ).respond_with_data("OK") - env = dict(os.environ, SENTRY_DSN=make_dsn(httpserver)) + tx_items = [ + item + for req, _ in httpserver.log + for item in Envelope.deserialize(req.get_data()).items + if item.headers.get("type") == "transaction" + ] + assert tx_items - with httpserver.wait(timeout=10) as waiting: - run( - tmp_path, - "sentry_example", - ["log", "open-transaction", "crash"], - expect_failure=True, - env=env, - ) - assert waiting.result + tx = tx_items[0].payload.json + assert tx["contexts"]["trace"]["status"] == "aborted" + spans = tx.get("spans", []) + # Every in-flight child is finished, not just the deepest. + for op in ("open.span", "open.grand.span"): + span = next((s for s in spans if s.get("op") == op), None) + assert span is not None, f"missing {op} in {[s.get('op') for s in spans]}" + assert span.get("status") == "aborted" + assert span.get("timestamp") - assert_trace_finish_on_crash(httpserver) + # The crash event nests under the deepest active span via matching + # trace_id + span_id. + event_items = [ + item + for req, _ in httpserver.log + for item in Envelope.deserialize(req.get_data()).items + if item.headers.get("type") == "event" + ] + assert event_items + event = event_items[0].payload.json + grand = next(s for s in spans if s.get("op") == "open.grand.span") + assert event["contexts"]["trace"]["trace_id"] == tx["contexts"]["trace"]["trace_id"] + assert event["contexts"]["trace"]["span_id"] == grand["span_id"] + assert event.get("level") == "fatal" @pytest.mark.skipif(not has_files, reason="test needs a local filesystem") From 39f28f32b336a17ba2622d175b9e827e8ad6351d Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 27 Apr 2026 14:13:37 +0200 Subject: [PATCH 15/20] fix: dump crash trace transactions separately --- src/backends/native/sentry_crash_daemon.c | 20 ------- src/backends/sentry_backend_breakpad.cpp | 20 +++---- src/backends/sentry_backend_inproc.c | 21 ++++--- src/backends/sentry_backend_native.c | 71 +++++++++-------------- src/sentry_core.c | 43 +++++--------- src/sentry_core.h | 3 - src/sentry_envelope.c | 5 +- tests/test_integration_http.py | 4 ++ 8 files changed, 66 insertions(+), 121 deletions(-) diff --git a/src/backends/native/sentry_crash_daemon.c b/src/backends/native/sentry_crash_daemon.c index 616ce1826..2fa0c58fc 100644 --- a/src/backends/native/sentry_crash_daemon.c +++ b/src/backends/native/sentry_crash_daemon.c @@ -2424,16 +2424,6 @@ write_envelope_with_native_stacktrace(const sentry_options_t *options, sentry_free(event_json); - if (run_folder) { - sentry_path_t *transaction_path - = sentry__path_join_str(run_folder, "__sentry-transaction"); - if (transaction_path) { - write_json_item_to_envelope( - fd, transaction_path->path, "transaction"); - sentry__path_free(transaction_path); - } - } - // Add minidump as attachment if provided if (minidump_path && minidump_path[0]) { #if defined(SENTRY_PLATFORM_UNIX) @@ -2687,16 +2677,6 @@ write_envelope_with_minidump(const sentry_options_t *options, } sentry_free(event_json); - if (run_folder) { - sentry_path_t *transaction_path - = sentry__path_join_str(run_folder, "__sentry-transaction"); - if (transaction_path) { - write_json_item_to_envelope( - fd, transaction_path->path, "transaction"); - sentry__path_free(transaction_path); - } - } - // Add minidump as attachment #if defined(SENTRY_PLATFORM_UNIX) int minidump_fd = open(minidump_path, O_RDONLY); diff --git a/src/backends/sentry_backend_breakpad.cpp b/src/backends/sentry_backend_breakpad.cpp index 76e5fa770..167bdd3a1 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -189,15 +189,6 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, sentry_envelope_t *envelope = sentry__prepare_event( options, event, nullptr, !options->on_crash_func, nullptr); - if (!sentry_value_is_null(transaction)) { - transaction = sentry__prepare_transaction_value( - options, transaction, nullptr); - if (!sentry_value_is_null(transaction) - && !sentry__envelope_add_transaction( - envelope, transaction)) { - sentry_value_decref(transaction); - } - } sentry_session_t *session = sentry__end_current_session_with_status( SENTRY_SESSION_STATUS_CRASHED); sentry__envelope_add_session(envelope, session); @@ -253,9 +244,18 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, } if (!sentry__launch_external_crash_reporter(options, envelope)) { - // capture the envelope with the disk transport + // capture the envelopes with the disk transport sentry_transport_t *disk_transport = sentry_new_disk_transport(options->run); + if (!sentry_value_is_null(transaction)) { + sentry_envelope_t *tx_envelope + = sentry__prepare_transaction( + options, transaction, nullptr); + if (tx_envelope) { + sentry__capture_envelope( + disk_transport, tx_envelope, options); + } + } sentry__capture_envelope(disk_transport, envelope, options); sentry__transport_dump_queue(disk_transport, options->run); sentry_transport_free(disk_transport); diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 3c0934ef0..2c9e9bd73 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -1148,16 +1148,6 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx, sentry_envelope_t *envelope = sentry__prepare_event(options, event, NULL, !options->on_crash_func && !skip_hooks, NULL); - if (!sentry_value_is_null(transaction)) { - transaction - = sentry__prepare_transaction_value(options, transaction, NULL); - if (!sentry_value_is_null(transaction) - && !sentry__envelope_add_transaction( - envelope, transaction)) { - sentry_value_decref(transaction); - } - } - sentry_session_t *session = sentry__end_current_session_with_status( SENTRY_SESSION_STATUS_CRASHED); sentry__envelope_add_session(envelope, session); @@ -1184,9 +1174,18 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx, } if (!sentry__launch_external_crash_reporter(options, envelope)) { - // capture the envelope with the disk transport + // capture the envelopes with the disk transport sentry_transport_t *disk_transport = sentry_new_disk_transport(options->run); + if (!sentry_value_is_null(transaction)) { + sentry_envelope_t *tx_envelope + = sentry__prepare_transaction( + options, transaction, NULL); + if (tx_envelope) { + sentry__capture_envelope( + disk_transport, tx_envelope, options); + } + } sentry__capture_envelope(disk_transport, envelope, options); sentry__transport_dump_queue(disk_transport, options->run); sentry_transport_free(disk_transport); diff --git a/src/backends/sentry_backend_native.c b/src/backends/sentry_backend_native.c index 62e70ae34..c566de21e 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -1087,32 +1087,6 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx) } } - if (!sentry_value_is_null(transaction) && state - && state->event_path) { - transaction = sentry__prepare_transaction_value( - options, transaction, NULL); - if (!sentry_value_is_null(transaction)) { - char *transaction_json - = sentry_value_to_json(transaction); - sentry_path_t *run_path - = sentry__path_dir(state->event_path); - sentry_path_t *transaction_path = run_path - ? sentry__path_join_str( - run_path, "__sentry-transaction") - : NULL; - if (transaction_json && transaction_path) { - sentry__path_write_buffer(transaction_path, - transaction_json, strlen(transaction_json)); - } - sentry__path_free(transaction_path); - sentry__path_free(run_path); - sentry_free(transaction_json); - sentry_value_decref(transaction); - } - } else { - sentry_value_decref(transaction); - } - sentry_value_decref(event); // End session with crashed status and write session envelope to @@ -1122,26 +1096,33 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx) = sentry__end_current_session_with_status( SENTRY_SESSION_STATUS_CRASHED); - if (session) { - sentry_envelope_t *envelope = sentry__envelope_new(); - if (envelope) { - sentry__envelope_add_session(envelope, session); - - // Write session envelope to disk - sentry_transport_t *disk_transport - = sentry_new_disk_transport(options->run); - if (disk_transport) { - // sentry__capture_envelope takes ownership of - // envelope - sentry__capture_envelope( - disk_transport, envelope, options); - sentry__transport_dump_queue( - disk_transport, options->run); - sentry_transport_free(disk_transport); - } else { - // Failed to create transport, free envelope - sentry_envelope_free(envelope); + if (session || !sentry_value_is_null(transaction)) { + sentry_transport_t *disk_transport + = sentry_new_disk_transport(options->run); + if (disk_transport) { + if (!sentry_value_is_null(transaction)) { + sentry_envelope_t *tx_envelope + = sentry__prepare_transaction( + options, transaction, NULL); + if (tx_envelope) { + sentry__capture_envelope( + disk_transport, tx_envelope, options); + } } + if (session) { + sentry_envelope_t *envelope + = sentry__envelope_new(); + if (envelope) { + sentry__envelope_add_session(envelope, session); + sentry__capture_envelope( + disk_transport, envelope, options); + } + } + sentry__transport_dump_queue( + disk_transport, options->run); + sentry_transport_free(disk_transport); + } else { + sentry_value_decref(transaction); } } diff --git a/src/sentry_core.c b/src/sentry_core.c index ac6fcd088..791020260 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -725,32 +725,6 @@ sentry__prepare_transaction(const sentry_options_t *options, { sentry_envelope_t *envelope = NULL; - transaction - = sentry__prepare_transaction_value(options, transaction, event_id); - if (sentry_value_is_null(transaction)) { - return NULL; - } - - envelope = sentry__envelope_new(); - if (!envelope || !sentry__envelope_add_transaction(envelope, transaction)) { - goto fail; - } - - // TODO(tracing): Revisit when adding attachment support for transactions. - - return envelope; - -fail: - SENTRY_WARN("dropping transaction"); - sentry_envelope_free(envelope); - sentry_value_decref(transaction); - return NULL; -} - -sentry_value_t -sentry__prepare_transaction_value(const sentry_options_t *options, - sentry_value_t transaction, sentry_uuid_t *event_id) -{ SENTRY_WITH_SCOPE (scope) { SENTRY_DEBUG("merging scope into transaction"); // Don't include debugging info @@ -768,12 +742,25 @@ sentry__prepare_transaction_value(const sentry_options_t *options, "transaction was discarded by the `before_transaction` hook"); sentry__client_report_discard(SENTRY_DISCARD_REASON_BEFORE_SEND, SENTRY_DATA_CATEGORY_TRANSACTION, 1); - return sentry_value_new_null(); + return NULL; } } sentry__ensure_event_id(transaction, event_id); - return transaction; + envelope = sentry__envelope_new(); + if (!envelope || !sentry__envelope_add_transaction(envelope, transaction)) { + goto fail; + } + + // TODO(tracing): Revisit when adding attachment support for transactions. + + return envelope; + +fail: + SENTRY_WARN("dropping transaction"); + sentry_envelope_free(envelope); + sentry_value_decref(transaction); + return NULL; } static sentry_envelope_t * diff --git a/src/sentry_core.h b/src/sentry_core.h index 352303371..ad3026e9b 100644 --- a/src/sentry_core.h +++ b/src/sentry_core.h @@ -88,9 +88,6 @@ sentry_uuid_t sentry__capture_event( */ sentry_envelope_t *sentry__prepare_transaction(const sentry_options_t *options, sentry_value_t transaction, sentry_uuid_t *event_id); -sentry_value_t sentry__prepare_transaction_value( - const sentry_options_t *options, sentry_value_t transaction, - sentry_uuid_t *event_id); sentry_value_t sentry__transaction_finish_value( sentry_transaction_t *opaque_tx, uint64_t timestamp); diff --git a/src/sentry_envelope.c b/src/sentry_envelope.c index ada0ea7c0..655904302 100644 --- a/src/sentry_envelope.c +++ b/src/sentry_envelope.c @@ -380,10 +380,7 @@ sentry__envelope_add_event(sentry_envelope_t *envelope, sentry_value_t event) sentry_value_t length = sentry_value_new_int32((int32_t)item->payload_len); sentry__envelope_item_set_header(item, "length", length); - sentry_uuid_t envelope_event_id = sentry__envelope_get_event_id(envelope); - if (sentry_uuid_is_nil(&envelope_event_id)) { - sentry__envelope_set_event_id(envelope, &event_id); - } + sentry__envelope_set_event_id(envelope, &event_id); double traces_sample_rate = 0.0; SENTRY_WITH_OPTIONS (options) { diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index 444640fa3..ca892c348 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -877,6 +877,10 @@ def test_trace_finish_on_crash(cmake, httpserver, backend): unfinished transaction on the scope ships alongside the crash.""" tmp_path = cmake(["sentry_example"], {"SENTRY_BACKEND": backend}) + httpserver.expect_oneshot_request( + "/api/123456/envelope/", + headers={"x-sentry-auth": auth_header}, + ).respond_with_data("OK") httpserver.expect_oneshot_request( "/api/123456/envelope/", headers={"x-sentry-auth": auth_header}, From 3abfaffb060ddbb9ff4d49df38c13365d7697f36 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 19:04:11 +0200 Subject: [PATCH 16/20] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 770d38626..173c196f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ **Fixes**: +- Finish active trace on crash. ([#1667](https://github.com/getsentry/sentry-native/pull/1667)) - Reject overly deep msgpack payloads during deserialization. ([#1727](https://github.com/getsentry/sentry-native/pull/1727)) - Read lengths for variadic fingerprints. ([#1730](https://github.com/getsentry/sentry-native/pull/1730)) - Guard against JSON token allocation overflow on 32-bit platforms. ([#1733](https://github.com/getsentry/sentry-native/pull/1733)) From 8847ef83f334bef3c41d77fe565d903c14d952c9 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 20 May 2026 16:40:46 +0200 Subject: [PATCH 17/20] ref: clean up unused function --- src/backends/native/sentry_crash_daemon.c | 38 ----------------------- 1 file changed, 38 deletions(-) diff --git a/src/backends/native/sentry_crash_daemon.c b/src/backends/native/sentry_crash_daemon.c index 2fa0c58fc..b440aa1d2 100644 --- a/src/backends/native/sentry_crash_daemon.c +++ b/src/backends/native/sentry_crash_daemon.c @@ -303,44 +303,6 @@ add_attachment_refs(sentry_envelope_t *envelope, sentry_value_decref(list); } -static bool -write_json_item_to_envelope(int fd, const char *file_path, const char *type) -{ - sentry_path_t *path = sentry__path_from_str(file_path); - size_t size = 0; - char *json = path ? sentry__path_read_to_buffer(path, &size) : NULL; - sentry__path_free(path); - if (!json || size == 0) { - sentry_free(json); - return false; - } - - char header[SENTRY_CRASH_ITEM_HEADER_SIZE]; - int header_len = snprintf( - header, sizeof(header), "{\"type\":\"%s\",\"length\":%zu}\n", type, size); - if (header_len <= 0 || header_len >= (int)sizeof(header)) { - sentry_free(json); - return false; - } - -#if defined(SENTRY_PLATFORM_UNIX) - if (write(fd, header, header_len) != (ssize_t)header_len - || write(fd, json, size) != (ssize_t)size - || write(fd, "\n", 1) != 1) { - SENTRY_WARNF("Failed to write %s item to envelope", type); - sentry_free(json); - return false; - } -#elif defined(SENTRY_PLATFORM_WINDOWS) - _write(fd, header, (unsigned int)header_len); - _write(fd, json, (unsigned int)size); - _write(fd, "\n", 1); -#endif - - sentry_free(json); - return true; -} - #if defined(SENTRY_PLATFORM_UNIX) /** * Get signal name from signal number (Unix platforms only) From e879be9a91703bda50d329f6e1b52f6c6debf914 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 21 May 2026 07:14:08 +0200 Subject: [PATCH 18/20] fix: preserve transaction root trace when merging scope --- src/sentry_scope.c | 15 +++++++--- tests/test_integration_http.py | 51 +++++++++++++++++++++++++++++----- tests/unit/test_tracing.c | 23 ++++++++++++++- 3 files changed, 77 insertions(+), 12 deletions(-) diff --git a/src/sentry_scope.c b/src/sentry_scope.c index 48ede5524..1c055e8ba 100644 --- a/src/sentry_scope.c +++ b/src/sentry_scope.c @@ -436,13 +436,20 @@ sentry__scope_apply_to_event(const sentry_scope_t *scope, sentry__value_merge_objects(event_extra, scope->extra); } + bool is_transaction = sentry__event_is_transaction(event); sentry_value_t contexts = sentry__value_clone(scope->contexts); + if (is_transaction && !sentry_value_is_null(contexts)) { + sentry_value_remove_by_key(contexts, "trace"); + } // prep contexts sourced from scope; data about transaction on scope needs // to be extracted and inserted - sentry_value_t scoped_txn_or_span = get_span_or_transaction(scope); - sentry_value_t scope_trace - = sentry__value_get_trace_context(scoped_txn_or_span); + sentry_value_t scoped_txn_or_span = sentry_value_new_null(); + sentry_value_t scope_trace = sentry_value_new_null(); + if (!is_transaction) { + scoped_txn_or_span = get_span_or_transaction(scope); + scope_trace = sentry__value_get_trace_context(scoped_txn_or_span); + } if (!sentry_value_is_null(scope_trace)) { if (sentry_value_is_null(contexts)) { contexts = sentry_value_new_object(); @@ -461,7 +468,7 @@ sentry__scope_apply_to_event(const sentry_scope_t *scope, sentry_value_t event_contexts = sentry_value_get_by_key(event, "contexts"); if (sentry_value_is_null(event_contexts)) { // only merge in propagation context if there is no scoped span - if (sentry_value_is_null(scope_trace)) { + if (!is_transaction && sentry_value_is_null(scope_trace)) { sentry__value_merge_objects(contexts, scope->propagation_context); } PLACE_VALUE("contexts", contexts); diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index ca892c348..e53b99c89 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -900,17 +900,45 @@ def test_trace_finish_on_crash(cmake, httpserver, backend): run(tmp_path, "sentry_example", ["log", "no-setup"], env=env) assert waiting.result + envelopes = [ + Envelope.deserialize(req.get_data()) + for req, _ in httpserver.log + ] + for envelope in envelopes: + item_types = [item.headers.get("type") for item in envelope.items] + assert not ("event" in item_types and "transaction" in item_types) + + tx_envelopes = [ + envelope + for envelope in envelopes + if any( + item.headers.get("type") == "transaction" + for item in envelope.items + ) + ] + assert tx_envelopes + tx_envelope = tx_envelopes[0] tx_items = [ item - for req, _ in httpserver.log - for item in Envelope.deserialize(req.get_data()).items + for item in tx_envelope.items if item.headers.get("type") == "transaction" ] assert tx_items tx = tx_items[0].payload.json - assert tx["contexts"]["trace"]["status"] == "aborted" + assert tx_envelope.headers["event_id"] == tx["event_id"] + tx_trace = tx["contexts"]["trace"] + assert tx_trace["status"] == "aborted" + assert "parent_span_id" not in tx_trace + tx_trace_header = tx_envelope.headers["trace"] + assert tx_trace_header["trace_id"] == tx_trace["trace_id"] + assert tx_trace_header["sampled"] == "true" + assert tx_trace_header["transaction"] == tx["transaction"] spans = tx.get("spans", []) + child = next(s for s in spans if s.get("op") == "open.span") + grand = next(s for s in spans if s.get("op") == "open.grand.span") + assert child["parent_span_id"] == tx_trace["span_id"] + assert grand["parent_span_id"] == child["span_id"] # Every in-flight child is finished, not just the deepest. for op in ("open.span", "open.grand.span"): span = next((s for s in spans if s.get("op") == op), None) @@ -920,16 +948,25 @@ def test_trace_finish_on_crash(cmake, httpserver, backend): # The crash event nests under the deepest active span via matching # trace_id + span_id. + event_envelopes = [ + envelope + for envelope in envelopes + if any(item.headers.get("type") == "event" for item in envelope.items) + ] + assert event_envelopes + event_envelope = event_envelopes[0] event_items = [ item - for req, _ in httpserver.log - for item in Envelope.deserialize(req.get_data()).items + for item in event_envelope.items if item.headers.get("type") == "event" ] assert event_items + if backend != "native": + event_trace_header = event_envelope.headers["trace"] + assert event_trace_header["trace_id"] == tx_trace["trace_id"] + assert event_trace_header["sampled"] == "true" event = event_items[0].payload.json - grand = next(s for s in spans if s.get("op") == "open.grand.span") - assert event["contexts"]["trace"]["trace_id"] == tx["contexts"]["trace"]["trace_id"] + assert event["contexts"]["trace"]["trace_id"] == tx_trace["trace_id"] assert event["contexts"]["trace"]["span_id"] == grand["span_id"] assert event.get("level") == "fatal" diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index 9c6047fbf..9301ce2a6 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -1,5 +1,6 @@ #include "sentry_testsupport.h" +#include "sentry_core.h" #include "sentry_options.h" #include "sentry_scope.h" #include "sentry_string.h" @@ -806,11 +807,15 @@ SENTRY_TEST(trace_finish) = sentry_transaction_context_new("root", "op"); sentry_transaction_t *tx = sentry_transaction_start(ctx, sentry_value_new_null()); + sentry_transaction_set_data( + tx, "tx-data", sentry_value_new_string("root")); sentry_set_transaction_object(tx); sentry_span_t *child = sentry_transaction_start_child(tx, "child-op", "child"); sentry_span_t *grand = sentry_span_start_child(child, "grand-op", "grand"); + sentry_span_set_data( + grand, "span-data", sentry_value_new_string("child")); sentry_set_span(grand); sentry_value_t finished = sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); @@ -827,7 +832,23 @@ SENTRY_TEST(trace_finish) TEST_CHECK( !sentry_value_is_null(sentry_value_get_by_key(span, "timestamp"))); } - sentry_value_decref(finished); + + sentry_envelope_t *envelope = NULL; + SENTRY_WITH_OPTIONS (runtime_options) { + envelope = sentry__prepare_transaction(runtime_options, finished, NULL); + } + TEST_ASSERT(envelope != NULL); + + sentry_value_t prepared = sentry_envelope_get_transaction(envelope); + sentry_value_t prepared_trace = sentry_value_get_by_key( + sentry_value_get_by_key(prepared, "contexts"), "trace"); + CHECK_STRING_PROPERTY(prepared_trace, "status", "aborted"); + TEST_CHECK(IS_NULL(prepared_trace, "parent_span_id")); + + sentry_value_t trace_data = sentry_value_get_by_key(prepared_trace, "data"); + CHECK_STRING_PROPERTY(trace_data, "tx-data", "root"); + TEST_CHECK(IS_NULL(trace_data, "span-data")); + sentry_envelope_free(envelope); // Scope still points at the (finished) span so a subsequent crash event // inherits its trace context. From 336c007aaef808a7f30e3b60a3e19b59c2fdf186 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 21 May 2026 07:15:11 +0200 Subject: [PATCH 19/20] make format --- tests/test_integration_http.py | 19 +++++-------------- tests/unit/test_tracing.c | 11 +++++------ 2 files changed, 10 insertions(+), 20 deletions(-) diff --git a/tests/test_integration_http.py b/tests/test_integration_http.py index e53b99c89..a3b728717 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -853,6 +853,7 @@ def test_native_crash_http(cmake, httpserver): assert_breadcrumb(envelope) assert_attachment(envelope) + @pytest.mark.parametrize( "backend", [ @@ -900,10 +901,7 @@ def test_trace_finish_on_crash(cmake, httpserver, backend): run(tmp_path, "sentry_example", ["log", "no-setup"], env=env) assert waiting.result - envelopes = [ - Envelope.deserialize(req.get_data()) - for req, _ in httpserver.log - ] + envelopes = [Envelope.deserialize(req.get_data()) for req, _ in httpserver.log] for envelope in envelopes: item_types = [item.headers.get("type") for item in envelope.items] assert not ("event" in item_types and "transaction" in item_types) @@ -911,17 +909,12 @@ def test_trace_finish_on_crash(cmake, httpserver, backend): tx_envelopes = [ envelope for envelope in envelopes - if any( - item.headers.get("type") == "transaction" - for item in envelope.items - ) + if any(item.headers.get("type") == "transaction" for item in envelope.items) ] assert tx_envelopes tx_envelope = tx_envelopes[0] tx_items = [ - item - for item in tx_envelope.items - if item.headers.get("type") == "transaction" + item for item in tx_envelope.items if item.headers.get("type") == "transaction" ] assert tx_items @@ -956,9 +949,7 @@ def test_trace_finish_on_crash(cmake, httpserver, backend): assert event_envelopes event_envelope = event_envelopes[0] event_items = [ - item - for item in event_envelope.items - if item.headers.get("type") == "event" + item for item in event_envelope.items if item.headers.get("type") == "event" ] assert event_items if backend != "native": diff --git a/tests/unit/test_tracing.c b/tests/unit/test_tracing.c index 9301ce2a6..741a4c94f 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -807,21 +807,20 @@ SENTRY_TEST(trace_finish) = sentry_transaction_context_new("root", "op"); sentry_transaction_t *tx = sentry_transaction_start(ctx, sentry_value_new_null()); - sentry_transaction_set_data( - tx, "tx-data", sentry_value_new_string("root")); + sentry_transaction_set_data(tx, "tx-data", sentry_value_new_string("root")); sentry_set_transaction_object(tx); sentry_span_t *child = sentry_transaction_start_child(tx, "child-op", "child"); sentry_span_t *grand = sentry_span_start_child(child, "grand-op", "grand"); - sentry_span_set_data( - grand, "span-data", sentry_value_new_string("child")); + sentry_span_set_data(grand, "span-data", sentry_value_new_string("child")); sentry_set_span(grand); sentry_value_t finished = sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED); TEST_CHECK(!sentry_value_is_null(finished)); - CHECK_STRING_PROPERTY(sentry_value_get_by_key( - sentry_value_get_by_key(finished, "contexts"), "trace"), + CHECK_STRING_PROPERTY( + sentry_value_get_by_key( + sentry_value_get_by_key(finished, "contexts"), "trace"), "status", "aborted"); sentry_value_t spans = sentry_value_get_by_key(finished, "spans"); From 30008f0a3aa041a3c70da07870a850bf67cd7e8c Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 22 May 2026 15:57:01 +0200 Subject: [PATCH 20/20] decref transactions --- src/backends/sentry_backend_breakpad.cpp | 2 ++ src/backends/sentry_backend_inproc.c | 2 ++ src/backends/sentry_backend_native.c | 2 ++ 3 files changed, 6 insertions(+) diff --git a/src/backends/sentry_backend_breakpad.cpp b/src/backends/sentry_backend_breakpad.cpp index 167bdd3a1..730db0156 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -259,6 +259,8 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, sentry__capture_envelope(disk_transport, envelope, options); sentry__transport_dump_queue(disk_transport, options->run); sentry_transport_free(disk_transport); + } else { + sentry_value_decref(transaction); } // now that the envelope was written, we can remove the temporary diff --git a/src/backends/sentry_backend_inproc.c b/src/backends/sentry_backend_inproc.c index 2c9e9bd73..c71a8a775 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -1189,6 +1189,8 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx, sentry__capture_envelope(disk_transport, envelope, options); sentry__transport_dump_queue(disk_transport, options->run); sentry_transport_free(disk_transport); + } else { + sentry_value_decref(transaction); } } else { SENTRY_DEBUG("event was discarded by the `on_crash` hook"); diff --git a/src/backends/sentry_backend_native.c b/src/backends/sentry_backend_native.c index c566de21e..b186e46f6 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -1131,6 +1131,8 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx) SENTRY_DEBUG("crash event and session written, daemon will " "create and send minidump"); + } else { + sentry_value_decref(transaction); } } else { SENTRY_DEBUG("event was discarded by the `on_crash` hook");