fix: deadlock in dynamic sdk-name scope init... (#858)

Author: supervacuus

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+**Fixes**:
+
+- Remove deadlock pattern in dynamic sdk-name assignment ([#858](https://github.com/getsentry/sentry-native/pull/858))
+
 ## 0.6.4
 
 **Fixes**:

CONTRIBUTING.md

@@ -144,6 +144,7 @@ The example currently supports the following commands:
 - `discarding-before-send`: Installs a `before_send()` callback that discards the event.
 - `on-crash`: Installs an `on_crash()` callback that retains the crash event. 
 - `discarding-on-crash`: Installs an `on_crash()` callback that discards the crash event.
+- `override-sdk-name`: Changes the SDK name via the options at runtime.
 
 Only on Windows using crashpad with its WER handler module: 
 

examples/example.c

@@ -218,6 +218,10 @@ main(int argc, char **argv)
             options, discarding_on_crash_callback, NULL);
     }
 
+    if (has_arg(argc, argv, "override-sdk-name")) {
+        sentry_options_set_sdk_name(options, "sentry.native.android.flutter");
+    }
+
     sentry_init(options);
 
     if (!has_arg(argc, argv, "no-setup")) {

src/sentry_core.c

@@ -163,11 +163,16 @@ sentry_init(sentry_options_t *options)
     g_options = options;
 
     // *after* setting the global options, trigger a scope and consent flush,
-    // since at least crashpad needs that.
-    // the only way to get a reference to the scope is by locking it, the macro
-    // does all that at once, including invoking the backends scope flush hook
+    // since at least crashpad needs that. At this point we also freeze the
+    // `client_sdk` in the `scope` because some downstream SDKs want to override
+    // it at runtime via the options interface.
     SENTRY_WITH_SCOPE_MUT (scope) {
-        (void)scope;
+        if (options->sdk_name) {
+            sentry_value_t sdk_name
+                = sentry_value_new_string(options->sdk_name);
+            sentry_value_set_by_key(scope->client_sdk, "name", sdk_name);
+        }
+        sentry_value_freeze(scope->client_sdk);
     }
     if (backend && backend->user_consent_changed_func) {
         backend->user_consent_changed_func(backend);

src/sentry_scope.c

@@ -29,15 +29,9 @@ get_client_sdk(void)
 {
     sentry_value_t client_sdk = sentry_value_new_object();
 
-    SENTRY_WITH_OPTIONS (options) {
-        sentry_value_t sdk_name = sentry_value_new_string(options->sdk_name);
-        sentry_value_set_by_key(client_sdk, "name", sdk_name);
-    }
-    // in case the SDK is not initialized yet, fallback to build-time value
-    if (sentry_value_is_null(sentry_value_get_by_key(client_sdk, "name"))) {
-        sentry_value_t sdk_name = sentry_value_new_string(SENTRY_SDK_NAME);
-        sentry_value_set_by_key(client_sdk, "name", sdk_name);
-    }
+    // the SDK is not initialized yet, fallback to build-time value
+    sentry_value_t sdk_name = sentry_value_new_string(SENTRY_SDK_NAME);
+    sentry_value_set_by_key(client_sdk, "name", sdk_name);
 
     sentry_value_t version = sentry_value_new_string(SENTRY_SDK_VERSION);
     sentry_value_set_by_key(client_sdk, "version", version);
@@ -61,7 +55,6 @@ get_client_sdk(void)
     sentry_value_set_by_key(client_sdk, "integrations", integrations);
 #endif
 
-    sentry_value_freeze(client_sdk);
     return client_sdk;
 }
 

tests/test_integration_stdout.py

@@ -46,6 +46,26 @@ def test_capture_stdout(cmake):
     assert_event(envelope)
 
 
+def test_dynamic_sdk_name_override(cmake):
+    tmp_path = cmake(
+        ["sentry_example"],
+        {
+            "SENTRY_BACKEND": "none",
+            "SENTRY_TRANSPORT": "none",
+        },
+    )
+
+    output = check_output(
+        tmp_path,
+        "sentry_example",
+        ["stdout", "override-sdk-name", "capture-event"],
+    )
+    envelope = Envelope.deserialize(output)
+
+    assert_meta(envelope, sdk_override="sentry.native.android.flutter")
+    assert_event(envelope)
+
+
 def test_sdk_name_override(cmake):
     sdk_name = "cUsToM.SDK"
     tmp_path = cmake(

tests/unit/test_concurrency.c

@@ -43,7 +43,7 @@ SENTRY_TEST(multiple_inits)
         SENTRY_LEVEL_INFO, "root", "Hello World!"));
 
     sentry_value_t obj = sentry_value_new_object();
-    // something that is not a uuid, as this will be forcibly changed
+    // something that is not a UUID, as this will be forcibly changed
     sentry_value_set_by_key(obj, "event_id", sentry_value_new_int32(1234));
     sentry_capture_event(obj);
 
@@ -64,7 +64,7 @@ thread_worker(void *called)
         SENTRY_LEVEL_INFO, "root", "Hello World!"));
 
     sentry_value_t obj = sentry_value_new_object();
-    // something that is not a uuid, as this will be forcibly changed
+    // something that is not a UUID, as this will be forcibly changed
     sentry_value_set_by_key(obj, "event_id", sentry_value_new_int32(1234));
     sentry_capture_event(obj);
 
@@ -75,7 +75,7 @@ SENTRY_TEST(concurrent_init)
 {
     long called = 0;
 
-#define THREADS_NUM 10
+#define THREADS_NUM 100
     sentry_threadid_t threads[THREADS_NUM];
 
     for (size_t i = 0; i < THREADS_NUM; i++) {

tests/unit/test_options.c

@@ -43,7 +43,7 @@ SENTRY_TEST(options_sdk_name_invalid)
     const char *sdk_name = NULL;
     const int result = sentry_options_set_sdk_name(options, sdk_name);
 
-    // then the value should should be ignored
+    // then the value should be ignored
     TEST_CHECK_INT_EQUAL(result, 1);
     TEST_CHECK_STRING_EQUAL(
         sentry_options_get_sdk_name(options), SENTRY_SDK_NAME);