perf: copy-on-write for sentry_value_t clone

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 **Fixes**:
 
+- Fix scope data loss from shared `sentry_value_t` containers while significantly improving scope merge performance with copy-on-write cloning. ([#1794](https://github.com/getsentry/sentry-native/pull/1794))
 - Crashpad: wait reliably for crash report uploads. ([#1885](https://github.com/getsentry/sentry-native/pull/1885))
 
 ## 0.15.4

src/sentry_scope.c

@@ -647,10 +647,10 @@ sentry__scope_apply_to_event(const sentry_scope_t *scope,
                     sentry_value_new_string(options->run->installation_id));
             }
         } else if (sentry_value_get_length(scope->user) > 0) {
-            PLACE_VALUE("user", scope->user);
+            PLACE_CLONED_VALUE("user", scope->user);
         }
     }
-    PLACE_VALUE("fingerprint", scope->fingerprint);
+    PLACE_CLONED_VALUE("fingerprint", scope->fingerprint);
     PLACE_STRING("transaction", scope->transaction);
     PLACE_VALUE("sdk", scope->client_sdk);
 

src/sentry_value.c

@@ -93,6 +93,7 @@ typedef struct {
     sentry_value_t *items;
     size_t len;
     size_t allocated;
+    long refcount;
 } list_t;
 
 typedef struct {
@@ -104,6 +105,7 @@ typedef struct {
     obj_pair_t *pairs;
     size_t len;
     size_t allocated;
+    long refcount;
 } obj_t;
 
 static const char *
@@ -161,33 +163,162 @@ thing_get_type(const thing_t *thing)
 }
 
 static void
-thing_free(thing_t *thing)
+list_free(list_t *list)
+{
+    if (sentry__atomic_fetch_and_add(&list->refcount, -1) != 1) {
+        return;
+    }
+    for (size_t i = 0; i < list->len; i++) {
+        sentry_value_decref(list->items[i]);
+    }
+    sentry_free(list->items);
+    sentry_free(list);
+}
+
+static void
+obj_free(obj_t *obj)
+{
+    if (sentry__atomic_fetch_and_add(&obj->refcount, -1) != 1) {
+        return;
+    }
+    for (size_t i = 0; i < obj->len; i++) {
+        sentry_free(obj->pairs[i].k);
+        sentry_value_decref(obj->pairs[i].v);
+    }
+    sentry_free(obj->pairs);
+    sentry_free(obj);
+}
+
+static list_t *
+list_clone(const list_t *list)
+{
+    list_t *clone = SENTRY_MAKE(list_t);
+    if (!clone) {
+        return NULL;
+    }
+    clone->len = list->len;
+    clone->allocated = list->len;
+    clone->refcount = 1;
+    if (list->len) {
+        clone->items = sentry_malloc(sizeof(sentry_value_t) * list->len);
+        if (!clone->items) {
+            sentry_free(clone);
+            return NULL;
+        }
+        memcpy(clone->items, list->items, sizeof(sentry_value_t) * list->len);
+        for (size_t i = 0; i < list->len; i++) {
+            sentry_value_incref(clone->items[i]);
+        }
+    }
+    return clone;
+}
+
+static obj_t *
+obj_clone(const obj_t *obj)
+{
+    obj_t *clone = SENTRY_MAKE(obj_t);
+    if (!clone) {
+        return NULL;
+    }
+    clone->allocated = obj->len;
+    clone->refcount = 1;
+    if (obj->len) {
+        clone->pairs = sentry_malloc(sizeof(obj_pair_t) * obj->len);
+        if (!clone->pairs) {
+            sentry_free(clone);
+            return NULL;
+        }
+        for (size_t i = 0; i < obj->len; i++) {
+            clone->pairs[i].k = sentry__string_clone(obj->pairs[i].k);
+            if (!clone->pairs[i].k) {
+                obj_free(clone);
+                return NULL;
+            }
+            clone->pairs[i].v = obj->pairs[i].v;
+            sentry_value_incref(clone->pairs[i].v);
+            clone->len++;
+        }
+    }
+    return clone;
+}
+
+static bool
+thing_detach(thing_t *thing)
 {
     switch (thing_get_type(thing)) {
     case THING_TYPE_LIST: {
-        list_t *list = thing->payload._ptr;
-        for (size_t i = 0; i < list->len; i++) {
-            sentry_value_decref(list->items[i]);
+        list_t *old = thing->payload._ptr;
+        if (sentry__atomic_fetch(&old->refcount) <= 1) {
+            return true;
         }
-        sentry_free(list->items);
-        sentry_free(list);
-        break;
+        list_t *clone = list_clone(old);
+        if (!clone) {
+            return false;
+        }
+        list_free(old);
+        thing->payload._ptr = clone;
+        return true;
     }
     case THING_TYPE_OBJECT: {
-        obj_t *obj = thing->payload._ptr;
-        for (size_t i = 0; i < obj->len; i++) {
-            sentry_free(obj->pairs[i].k);
-            sentry_value_decref(obj->pairs[i].v);
+        obj_t *old = thing->payload._ptr;
+        if (sentry__atomic_fetch(&old->refcount) <= 1) {
+            return true;
         }
-        sentry_free(obj->pairs);
-        sentry_free(obj);
+        obj_t *clone = obj_clone(old);
+        if (!clone) {
+            return false;
+        }
+        obj_free(old);
+        thing->payload._ptr = clone;
+        return true;
+    }
+    default:
+        return true;
+    }
+}
+
+static sentry_value_t
+thing_get_child(const thing_t *thing, size_t i)
+{
+    switch (thing_get_type(thing)) {
+    case THING_TYPE_LIST:
+        return ((const list_t *)thing->payload._ptr)->items[i];
+    case THING_TYPE_OBJECT:
+        return ((const obj_t *)thing->payload._ptr)->pairs[i].v;
+    default:
+        return sentry_value_new_null();
+    }
+}
+
+static void
+thing_set_child(thing_t *thing, size_t i, sentry_value_t value)
+{
+    switch (thing_get_type(thing)) {
+    case THING_TYPE_LIST:
+        ((list_t *)thing->payload._ptr)->items[i] = value;
+        break;
+    case THING_TYPE_OBJECT:
+        ((obj_t *)thing->payload._ptr)->pairs[i].v = value;
+        break;
+    default:
         break;
     }
-    case THING_TYPE_STRING: {
+}
+
+static void
+thing_free(thing_t *thing)
+{
+    switch (thing_get_type(thing)) {
+    case THING_TYPE_LIST:
+        list_free(thing->payload._ptr);
+        break;
+    case THING_TYPE_OBJECT:
+        obj_free(thing->payload._ptr);
+        break;
+    case THING_TYPE_STRING:
         sentry_free(thing->payload._ptr);
         break;
     }
-    }
     sentry_free(thing);
 }
 
@@ -395,6 +526,7 @@ sentry_value_new_list(void)
 {
     list_t *l = SENTRY_MAKE(list_t);
     if (l) {
+        l->refcount = 1;
         sentry_value_t rv = new_thing_value(l, THING_TYPE_LIST);
         if (sentry_value_is_null(rv)) {
             sentry_free(l);
@@ -410,6 +542,7 @@ sentry__value_new_list_with_size(size_t size)
 {
     list_t *l = SENTRY_MAKE(list_t);
     if (l) {
+        l->refcount = 1;
         l->allocated = size;
         if (size) {
             l->items = sentry_malloc(sizeof(sentry_value_t) * size);
@@ -434,6 +567,7 @@ sentry_value_new_object(void)
 {
     obj_t *o = SENTRY_MAKE(obj_t);
     if (o) {
+        o->refcount = 1;
         sentry_value_t rv = new_thing_value(o, THING_TYPE_OBJECT);
         if (sentry_value_is_null(rv)) {
             sentry_free(o);
@@ -449,6 +583,7 @@ sentry__value_new_object_with_size(size_t size)
 {
     obj_t *o = SENTRY_MAKE(obj_t);
     if (o) {
+        o->refcount = 1;
         o->allocated = size;
         if (size) {
             o->pairs = sentry_malloc(sizeof(obj_pair_t) * size);
@@ -639,7 +774,8 @@ sentry__value_set_by_key_owned(
     }
     sentry_slice_t k_slice = { k, k_len };
     thing_t *thing = value_as_unfrozen_thing(value);
-    if (!thing || thing_get_type(thing) != THING_TYPE_OBJECT) {
+    if (!thing || thing_get_type(thing) != THING_TYPE_OBJECT
+        || !thing_detach(thing)) {
         goto fail;
     }
     obj_t *o = thing->payload._ptr;
@@ -699,7 +835,8 @@ sentry__value_remove_and_take_key_n(
     }
     sentry_slice_t k_slice = { k, k_len };
     thing_t *thing = value_as_unfrozen_thing(value);
-    if (!thing || thing_get_type(thing) != THING_TYPE_OBJECT) {
+    if (!thing || thing_get_type(thing) != THING_TYPE_OBJECT
+        || !thing_detach(thing)) {
         return NULL;
     }
     obj_t *o = thing->payload._ptr;
@@ -742,7 +879,8 @@ int
 sentry_value_append(sentry_value_t value, sentry_value_t v)
 {
     thing_t *thing = value_as_unfrozen_thing(value);
-    if (!thing || thing_get_type(thing) != THING_TYPE_LIST) {
+    if (!thing || thing_get_type(thing) != THING_TYPE_LIST
+        || !thing_detach(thing)) {
         goto fail;
     }
 
@@ -811,6 +949,42 @@ sentry__value_stringify(sentry_value_t value)
 #undef STRINGIFY_NUMERIC
 }
 
+static bool
+value_is_container(sentry_value_t value)
+{
+    const thing_t *thing = value_as_thing(value);
+    if (!thing) {
+        return false;
+    }
+    int type = thing_get_type(thing);
+    return type == THING_TYPE_LIST || type == THING_TYPE_OBJECT;
+}
+
+static bool
+thing_clone_children(thing_t *thing, size_t len)
+{
+    bool detached = false;
+    for (size_t i = 0; i < len; i++) {
+        sentry_value_t child = thing_get_child(thing, i);
+        if (!value_is_container(child)) {
+            continue;
+        }
+        if (!detached) {
+            if (!thing_detach(thing)) {
+                return false;
+            }
+            detached = true;
+        }
+        sentry_value_t cloned = sentry__value_clone(child);
+        if (sentry_value_is_null(cloned)) {
+            return false;
+        }
+        sentry_value_decref(child);
+        thing_set_child(thing, i, cloned);
+    }
+    return true;
+}
+
 sentry_value_t
 sentry__value_clone(sentry_value_t value)
 {
@@ -820,20 +994,30 @@ sentry__value_clone(sentry_value_t value)
     }
     switch (thing_get_type(thing)) {
     case THING_TYPE_LIST: {
-        const list_t *list = thing->payload._ptr;
-        sentry_value_t rv = sentry__value_new_list_with_size(list->len);
-        for (size_t i = 0; i < list->len; i++) {
-            sentry_value_incref(list->items[i]);
-            sentry_value_append(rv, list->items[i]);
+        list_t *list = thing->payload._ptr;
+        sentry__atomic_fetch_and_add(&list->refcount, 1);
+        sentry_value_t rv = new_thing_value(list, THING_TYPE_LIST);
+        if (sentry_value_is_null(rv)) {
+            list_free(list);
+            return rv;
+        }
+        if (!thing_clone_children(value_as_thing(rv), list->len)) {
+            sentry_value_decref(rv);
+            return sentry_value_new_null();
         }
         return rv;
     }
     case THING_TYPE_OBJECT: {
-        const obj_t *obj = thing->payload._ptr;
-        sentry_value_t rv = sentry__value_new_object_with_size(obj->len);
-        for (size_t i = 0; i < obj->len; i++) {
-            sentry_value_incref(obj->pairs[i].v);
-            sentry_value_set_by_key(rv, obj->pairs[i].k, obj->pairs[i].v);
+        obj_t *obj = thing->payload._ptr;
+        sentry__atomic_fetch_and_add(&obj->refcount, 1);
+        sentry_value_t rv = new_thing_value(obj, THING_TYPE_OBJECT);
+        if (sentry_value_is_null(rv)) {
+            obj_free(obj);
+            return rv;
+        }
+        if (!thing_clone_children(value_as_thing(rv), obj->len)) {
+            sentry_value_decref(rv);
+            return sentry_value_new_null();
         }
         return rv;
     }
@@ -852,7 +1036,8 @@ int
 sentry_value_set_by_index(sentry_value_t value, size_t index, sentry_value_t v)
 {
     thing_t *thing = value_as_unfrozen_thing(value);
-    if (!thing || thing_get_type(thing) != THING_TYPE_LIST) {
+    if (!thing || thing_get_type(thing) != THING_TYPE_LIST
+        || !thing_detach(thing)) {
         goto fail;
     }
 
@@ -882,7 +1067,8 @@ int
 sentry_value_remove_by_index(sentry_value_t value, size_t index)
 {
     thing_t *thing = value_as_unfrozen_thing(value);
-    if (!thing || thing_get_type(thing) != THING_TYPE_LIST) {
+    if (!thing || thing_get_type(thing) != THING_TYPE_LIST
+        || !thing_detach(thing)) {
         return 1;
     }