ref(value)!: atomic decref result check

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+**Breaking**:
+
+- `sentry_value_incref` now returns `sentry_value_t` and `sentry_value_decref` returns `int` (0 if freed). ([#1763](https://github.com/getsentry/sentry-native/pull/1763))
+
 **Features**:
 
 - Add a `transfer_timeout` option for SDK-managed HTTP transports. ([#1741](https://github.com/getsentry/sentry-native/pull/1741))
@@ -21,6 +25,7 @@
 - Cap rate-limit retry-after values at 24 hours to prevent a MITM-provided response from disabling event delivery for the process lifetime. ([#1744](https://github.com/getsentry/sentry-native/pull/1744))
 - Native: validate ELF header entry sizes. ([#1746](https://github.com/getsentry/sentry-native/pull/1746))
 - Structured logs: respect printf argument widths when extracting log parameters to avoid stack-data disclosure and corrupted attributes on 32-bit platforms. ([#1752](https://github.com/getsentry/sentry-native/pull/1752))
+- Fix TOCTOU races in transaction/span refcounting by switching to the atomic decref return value. ([#1763](https://github.com/getsentry/sentry-native/pull/1763))
 
 ## 0.14.2
 

include/sentry.h

@@ -245,12 +245,14 @@ typedef union sentry_value_u sentry_value_t;
 /**
  * Increments the reference count on the value.
  */
-SENTRY_API void sentry_value_incref(sentry_value_t value);
+SENTRY_API sentry_value_t sentry_value_incref(sentry_value_t value);
 
 /**
  * Decrements the reference count on the value.
+ * Returns 0 if the value was freed or is a primitive (no tracking needed),
+ * or non-zero if it still has references.
  */
-SENTRY_API void sentry_value_decref(sentry_value_t value);
+SENTRY_API int sentry_value_decref(sentry_value_t value);
 
 /**
  * Returns the refcount of a value.

src/sentry_tracing.c

@@ -152,11 +152,8 @@ sentry__transaction_context_free(sentry_transaction_context_t *tx_ctx)
     if (!tx_ctx) {
         return;
     }
-    if (sentry_value_refcount(tx_ctx->inner) <= 1) {
-        sentry_value_decref(tx_ctx->inner);
+    if (!sentry_value_decref(tx_ctx->inner)) {
         sentry_free(tx_ctx);
-    } else {
-        sentry_value_decref(tx_ctx->inner);
     }
 }
 
@@ -475,13 +472,10 @@ sentry__transaction_decref(sentry_transaction_t *tx)
         return;
     }
 
-    if (sentry_value_refcount(tx->inner) <= 1) {
-        sentry_value_decref(tx->inner);
+    if (!sentry_value_decref(tx->inner)) {
         sentry_free(tx->children);
         sentry__mutex_free(&tx->children_mutex);
         sentry_free(tx);
-    } else {
-        sentry_value_decref(tx->inner);
     }
 }
 
@@ -516,13 +510,10 @@ sentry__span_decref(sentry_span_t *span)
         return;
     }
 
-    if (sentry_value_refcount(span->inner) <= 1) {
+    if (!sentry_value_decref(span->inner)) {
         sentry__transaction_remove_child(span->transaction, span);
-        sentry_value_decref(span->inner);
         sentry__transaction_decref(span->transaction);
         sentry_free(span);
-    } else {
-        sentry_value_decref(span->inner);
     }
 }
 

src/sentry_value.c

@@ -257,22 +257,25 @@ value_as_unfrozen_thing(sentry_value_t value)
 
 /* public api implementations */
 
-void
+sentry_value_t
 sentry_value_incref(sentry_value_t value)
 {
     thing_t *thing = value_as_thing(value);
     if (thing) {
         sentry__atomic_fetch_and_add(&thing->refcount, 1);
     }
+    return value;
 }
 
-void
+int
 sentry_value_decref(sentry_value_t value)
 {
     thing_t *thing = value_as_thing(value);
     if (thing && sentry__atomic_fetch_and_add(&thing->refcount, -1) == 1) {
         thing_free(thing);
+        return 0;
     }
+    return thing ? 1 : 0;
 }
 
 size_t

tests/unit/test_value.c

@@ -1762,3 +1762,33 @@ SENTRY_TEST(value_merge_breadcrumbs_max_limit)
     sentry_value_decref(list_a);
     sentry_value_decref(list_b);
 }
+
+SENTRY_TEST(value_refcount)
+{
+    // primitive values always report refcount 1, and decref is a no-op
+    sentry_value_t null_val = sentry_value_new_null();
+    TEST_CHECK_INT_EQUAL(1, sentry_value_refcount(null_val));
+    TEST_CHECK(!sentry_value_decref(null_val));
+    TEST_CHECK_INT_EQUAL(1, sentry_value_refcount(null_val));
+
+    sentry_value_t bool_val = sentry_value_new_bool(true);
+    TEST_CHECK_INT_EQUAL(1, sentry_value_refcount(bool_val));
+    TEST_CHECK(!sentry_value_decref(bool_val));
+    TEST_CHECK_INT_EQUAL(1, sentry_value_refcount(bool_val));
+
+    sentry_value_t int_val = sentry_value_new_int32(42);
+    TEST_CHECK_INT_EQUAL(1, sentry_value_refcount(int_val));
+    TEST_CHECK(!sentry_value_decref(int_val));
+    TEST_CHECK_INT_EQUAL(1, sentry_value_refcount(int_val));
+
+    // thing-allocated values track refcount through incref/decref
+    sentry_value_t obj = sentry_value_new_object();
+    TEST_CHECK_INT_EQUAL(1, sentry_value_refcount(obj));
+    TEST_CHECK_INT_EQUAL(2, sentry_value_refcount(sentry_value_incref(obj)));
+    TEST_CHECK_INT_EQUAL(3, sentry_value_refcount(sentry_value_incref(obj)));
+    TEST_CHECK(!!sentry_value_decref(obj));
+    TEST_CHECK_INT_EQUAL(2, sentry_value_refcount(obj));
+    TEST_CHECK(!!sentry_value_decref(obj));
+    TEST_CHECK_INT_EQUAL(1, sentry_value_refcount(obj));
+    TEST_CHECK(!sentry_value_decref(obj));
+}

tests/unit/tests.inc

@@ -383,6 +383,7 @@ XX(value_null)
 XX(value_object)
 XX(value_object_merge)
 XX(value_object_merge_nested)
+XX(value_refcount)
 XX(value_remove_by_null_key)
 XX(value_set_by_null_key)
 XX(value_set_stacktrace)