WIP: fix(tracing): finish active trace on crash

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
 

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,20 @@ main(int argc, char **argv)
         fflush(stdout);
     }
 
+    if (has_arg(argc, argv, "open-transaction")) {
+        // 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
+            = sentry_transaction_start(otx_ctx, sentry_value_new_null());
+        sentry_set_transaction_object(otx);
+        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")) {
         trigger_crash();
     }

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"
@@ -129,6 +130,9 @@ breakpad_backend_callback(const google_breakpad::MinidumpDescriptor &descriptor,
     SENTRY_WITH_OPTIONS (options) {
         sentry__write_crash_marker(options);
 
+        sentry_value_t transaction
+            = sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED);
+
         bool should_handle = true;
 
         if (options->on_crash_func) {
@@ -202,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);
@@ -218,6 +230,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

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"
@@ -1156,6 +1157,9 @@ process_ucontext_deferred(const sentry_ucontext_t *uctx,
         bool should_handle = true;
         sentry__write_crash_marker(options);
 
+        sentry_value_t transaction
+            = 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);
@@ -1185,9 +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);
-            // TODO(tracing): Revisit when investigating transaction flushing
-            //                during hard crashes.
-
             sentry_session_t *session = sentry__end_current_session_with_status(
                 SENTRY_SESSION_STATUS_CRASHED);
             sentry__envelope_add_session(envelope, session);
@@ -1203,16 +1204,24 @@ 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);
             }
         } 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

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"
 
@@ -847,6 +848,9 @@ native_backend_except(sentry_backend_t *backend, const sentry_ucontext_t *uctx)
         // Write crash marker
         sentry__write_crash_marker(options);
 
+        sentry_value_t transaction
+            = sentry__trace_finish(SENTRY_SPAN_STATUS_ABORTED);
+
         // Create crash event
         sentry_value_t event = sentry_value_new_event();
         sentry_value_set_by_key(
@@ -948,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);
                     }
                 }
 
@@ -980,6 +991,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);
         }
     }
 }

src/sentry_core.c

@@ -1302,6 +1302,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 +1396,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
@@ -1542,6 +1554,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(

src/sentry_core.h

@@ -89,6 +89,9 @@ 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__transaction_finish_value(
+    sentry_transaction_t *opaque_tx, uint64_t timestamp);
+
 /**
  * This function will submit the `envelope` to the given `transport`, first
  * checking for consent.

src/sentry_sampling_context.h

@@ -2,7 +2,8 @@
 #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;