From 87d7ace2a8d724f4036cfc937ec54b5c0ddf0ab9 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 15:18:29 +0200 Subject: [PATCH 01/16] 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 afcc5a5f73..4f4db27ed6 100644 --- a/examples/example.c +++ b/examples/example.c @@ -570,7 +570,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); } @@ -1008,6 +1009,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 20d9f878fb..7705ae090d 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -129,6 +129,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 6b7d6b395d..c64144b895 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -1156,6 +1156,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 9d43d6c3a4..aac65dddd5 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -847,6 +847,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 8ef7690bc6..94f77ac3a4 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -831,3 +831,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 f72836f6f0..950a3f1b17 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 a8a3a35656..34a699c27e 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -852,6 +852,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 96843011b1..e73db056f2 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -776,6 +776,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 8d8a37981b..463c451b04 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -264,6 +264,7 @@ XX(string_address_format) XX(symbolizer) XX(task_queue) XX(thread_without_name_still_valid) +XX(trace_finish) XX(traceparent_header_disabled_by_default) XX(traceparent_header_generation) XX(transaction_name_backfill_on_finish) From 47e5333ac3fb44760e45faebd6f52b0300d83eb9 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 17:55:55 +0200 Subject: [PATCH 02/16] 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 4f4db27ed6..a8c6fb13cc 100644 --- a/examples/example.c +++ b/examples/example.c @@ -1010,15 +1010,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 7705ae090d..0be8016583 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -18,6 +18,7 @@ extern "C" { #include "sentry_screenshot.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 c64144b895..8da78584d5 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -18,6 +18,7 @@ #include "sentry_scope.h" #include "sentry_screenshot.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 aac65dddd5..425c9e1c3a 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -34,6 +34,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 36d0b5b69b..9e5585fcb2 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -1542,6 +1542,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 59a069258d..6977f52cb7 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 94f77ac3a4..9619e8a493 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -336,6 +336,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; } @@ -357,12 +361,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) { @@ -404,6 +434,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; } @@ -835,29 +886,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 950a3f1b17..0a3a86c252 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 34a699c27e..6ae23da595 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -910,7 +910,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 e73db056f2..54596085b2 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -788,6 +788,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); } @@ -814,14 +825,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 682f690060c33c96aea0d008ad20c4e3ae22cc95 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 18:18:53 +0200 Subject: [PATCH 03/16] 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 a8c6fb13cc..74d0bfd038 100644 --- a/examples/example.c +++ b/examples/example.c @@ -1010,10 +1010,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 6977f52cb7..3d7ec52538 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 9619e8a493..b1d8fa6f4e 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -447,6 +447,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) { @@ -886,12 +888,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; @@ -918,9 +918,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; @@ -941,10 +940,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 6ae23da595..d5bee0de3b 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -911,17 +911,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 54596085b2..d847cef386 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -788,8 +788,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++) { @@ -831,18 +830,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 973c30d73a05d6cfe75b26fdbd5ab7c0e7068fc0 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 18:58:32 +0200 Subject: [PATCH 04/16] 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 b1d8fa6f4e..5dee698436 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -885,71 +885,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 cb567430ab8924f525d43a94f40015cd1401cf69 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 19:19:40 +0200 Subject: [PATCH 05/16] 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 5dee698436..a3499d1056 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -929,7 +929,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 2691500bc24aac84b05f16c1ffabb86a34124580 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 19:28:03 +0200 Subject: [PATCH 06/16] 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 a3499d1056..22068f3f75 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -949,7 +949,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 60418f0312d6eacb7e5a58ffe60f115da31a2491 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 19:55:15 +0200 Subject: [PATCH 07/16] 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 22068f3f75..5367696327 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -361,9 +361,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); @@ -378,19 +375,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 @@ -409,6 +401,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); @@ -452,7 +445,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 0a3a86c252..73555d283a 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 177c83fa44904c61f9aefaf32c03630f384ef852 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 21:22:33 +0200 Subject: [PATCH 08/16] 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 5367696327..0d1d57af07 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -939,6 +939,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 d847cef386..4e46b94f8f 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -826,7 +826,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); @@ -836,6 +837,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 364f1cc31857bc9c6e147a900307a6158fad4cba Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 24 Apr 2026 06:57:15 +0200 Subject: [PATCH 09/16] 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 0d1d57af07..6cb9278ed5 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -909,7 +909,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 93f41bbfeb829c0010cc27bf219df6992ff50757 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 24 Apr 2026 07:23:18 +0200 Subject: [PATCH 10/16] 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 6cb9278ed5..84f655410f 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -955,6 +955,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 92e4b3c8592da61e84bd89039b153b3ce34fd3c6 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 24 Apr 2026 11:36:07 +0200 Subject: [PATCH 11/16] 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 84f655410f..916e8d7eea 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -909,7 +909,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 264a69b5695094e65e0d80c7acecd7d6613c47eb Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Thu, 23 Apr 2026 19:04:11 +0200 Subject: [PATCH 12/16] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 313f4dfc30..003e48505b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ **Fixes**: - Fix event ownership (potential double-decref) in sentry_capture_minidump. ([#1669](https://github.com/getsentry/sentry-native/pull/1669)) +- Finish active trace on crash. ([#1667](https://github.com/getsentry/sentry-native/pull/1667)) ## 0.13.8 From ba3de42278588577adbcd145e99fb684d6c0ac8e Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 27 Apr 2026 12:38:48 +0200 Subject: [PATCH 13/16] 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 d5bee0de3b..3580e46b7b 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -852,6 +852,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", [ @@ -862,13 +897,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): @@ -876,7 +904,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}, @@ -895,43 +923,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 3266a8e747bb7ef7a6e206883984e1b02365202a Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 27 Apr 2026 13:39:03 +0200 Subject: [PATCH 14/16] 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 a44922795a..736d0a7911 100644 --- a/src/backends/native/sentry_crash_daemon.c +++ b/src/backends/native/sentry_crash_daemon.c @@ -194,6 +194,44 @@ write_attachment_to_envelope(int fd, const char *file_path, return true; } +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) @@ -2277,6 +2315,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) @@ -2513,6 +2561,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 0be8016583..e346843c5a 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -130,7 +130,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; @@ -178,6 +179,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); @@ -221,6 +231,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 8da78584d5..b044df8e10 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -1157,7 +1157,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"); @@ -1188,8 +1189,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); @@ -1216,6 +1224,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 425c9e1c3a..ea20bcd638 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -848,7 +848,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(); @@ -942,6 +943,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 @@ -983,6 +1010,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 9e5585fcb2..0144cdb228 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -748,6 +748,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 @@ -765,25 +791,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 * @@ -1302,6 +1315,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"); @@ -1382,12 +1409,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 c0b6f60fbf..352303371a 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 b292c6cd10..95a6a9ce14 100644 --- a/src/sentry_envelope.c +++ b/src/sentry_envelope.c @@ -370,7 +370,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 916e8d7eea..c43da69f17 100644 --- a/src/sentry_tracing.c +++ b/src/sentry_tracing.c @@ -946,7 +946,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 @@ -956,14 +956,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 73555d283a..44e8b23a05 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 3580e46b7b..3ed710c98d 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -904,11 +904,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}, @@ -936,16 +931,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 4e46b94f8f..680ad074c9 100644 --- a/tests/unit/test_tracing.c +++ b/tests/unit/test_tracing.c @@ -777,42 +777,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); @@ -829,7 +811,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. @@ -842,8 +838,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 ceb302a898e878e14ff0c28957a0c6dd08650e72 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 27 Apr 2026 13:53:55 +0200 Subject: [PATCH 15/16] 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 3ed710c98d..8f2b234778 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -851,42 +851,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", [ @@ -897,6 +861,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): @@ -918,36 +889,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 6a2e71b97990184d3efeb4fcc76b563f486d4ac5 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 27 Apr 2026 14:13:37 +0200 Subject: [PATCH 16/16] fix: dump crash trace transactions separately --- src/backends/native/sentry_crash_daemon.c | 58 ------------------ src/backends/sentry_backend_breakpad.cpp | 19 +++--- src/backends/sentry_backend_inproc.c | 19 +++--- 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, 63 insertions(+), 159 deletions(-) diff --git a/src/backends/native/sentry_crash_daemon.c b/src/backends/native/sentry_crash_daemon.c index 736d0a7911..a44922795a 100644 --- a/src/backends/native/sentry_crash_daemon.c +++ b/src/backends/native/sentry_crash_daemon.c @@ -194,44 +194,6 @@ write_attachment_to_envelope(int fd, const char *file_path, return true; } -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) @@ -2315,16 +2277,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) @@ -2561,16 +2513,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 e346843c5a..5506ebd259 100644 --- a/src/backends/sentry_backend_breakpad.cpp +++ b/src/backends/sentry_backend_breakpad.cpp @@ -179,15 +179,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); @@ -215,9 +206,17 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor, } if (!sentry__launch_external_crash_reporter(options, envelope)) { - // capture the envelope 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 b044df8e10..ea3b8e7188 100644 --- a/src/backends/sentry_backend_inproc.c +++ b/src/backends/sentry_backend_inproc.c @@ -1189,16 +1189,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); @@ -1214,9 +1204,16 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx, } if (!sentry__launch_external_crash_reporter(options, envelope)) { - // capture the envelope 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 ea20bcd638..5ec7c2ceaa 100644 --- a/src/backends/sentry_backend_native.c +++ b/src/backends/sentry_backend_native.c @@ -943,32 +943,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 @@ -978,26 +952,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 0144cdb228..65085ece33 100644 --- a/src/sentry_core.c +++ b/src/sentry_core.c @@ -748,32 +748,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 @@ -791,12 +765,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 352303371a..ad3026e9bf 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 95a6a9ce14..b292c6cd10 100644 --- a/src/sentry_envelope.c +++ b/src/sentry_envelope.c @@ -370,10 +370,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 8f2b234778..60f45e8693 100644 --- a/tests/test_integration_http.py +++ b/tests/test_integration_http.py @@ -875,6 +875,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},