fix: Don't overwrite MainThreadId

CHANGELOG.md

@@ -6,6 +6,10 @@
 
 - The SDK no longer attaches screenshots when capturing errors in the Unity Editor. ([#2163](https://github.com/getsentry/sentry-unity/pull/2163))
 
+## Fixes
+
+- Fixed a potential race condition that when targeting Android could cause 'attempting to detach while still running code' crashes ([#2165](https://github.com/getsentry/sentry-unity/pull/2165))
+
 ### Dependencies
 
 - Bump Java SDK from v8.11.1 to v8.12.0 ([#2155](https://github.com/getsentry/sentry-unity/pull/2155))

package-dev/Runtime/SentryInitialization.cs

@@ -92,7 +92,7 @@ private static void SetupNativeSdk(SentryUnityOptions options, SentryUnityInfo u
 #elif SENTRY_NATIVE
                 SentryNative.Configure(options, unityInfo);
 #elif SENTRY_WEBGL
-              SentryWebGL.Configure(options);
+                SentryWebGL.Configure(options);
 #endif
             }
             catch (DllNotFoundException e)

samples/unity-of-bugs/Assets/Scripts/BugFarmButtons.cs

@@ -1,6 +1,8 @@
 using System;
 using System.Globalization;
 using System.Runtime.CompilerServices;
+using System.Threading;
+using System.Threading.Tasks;
 using Sentry;
 using UnityEngine;
 using UnityEngine.Assertions;
@@ -15,7 +17,9 @@ private void Awake()
     private void Start()
     {
         Debug.Log("Sample Start 🦋");
-        Debug.LogWarning("Here come the bugs 🐞🦋🐛🐜🕷!");
+        Debug.LogWarning("Here come the bugs 🐞🦋🐛🐜!");
+
+        Task.Run(() => { Debug.Log("The spider snuck up from a task! 🕷"); });
     }
 
     public void AssertFalse() => Assert.AreEqual(true, false);

src/Sentry.Unity/MainThreadData.cs

@@ -1,6 +1,5 @@
 using System;
 using System.Threading;
-using UnityEngine;
 
 namespace Sentry.Unity;
 
@@ -69,8 +68,16 @@ internal static class MainThreadData
 
     public static DateTimeOffset? StartTime { get; set; }
 
-    public static bool IsMainThread()
-        => MainThreadId.HasValue && Thread.CurrentThread.ManagedThreadId == MainThreadId;
+    public static bool? IsMainThread()
+    {
+        if (MainThreadId.HasValue)
+        {
+            return MainThreadId.Equals(Thread.CurrentThread.ManagedThreadId);
+        }
+
+        // We don't know whether this is the main thread or not
+        return null;
+    }
 
     // For testing
     internal static ISentrySystemInfo? SentrySystemInfo { get; set; }
@@ -79,7 +86,9 @@ public static void CollectData()
     {
         var sentrySystemInfo = SentrySystemInfo ?? SentrySystemInfoAdapter.Instance;
 
-        MainThreadId = sentrySystemInfo.MainThreadId;
+        // Don't overwrite the MainThreadId if it's already been set
+        MainThreadId ??= sentrySystemInfo.MainThreadId;
+
         ProcessorCount = sentrySystemInfo.ProcessorCount;
         OperatingSystem = sentrySystemInfo.OperatingSystem;
         CpuDescription = sentrySystemInfo.CpuDescription;

src/Sentry.Unity/ScreenshotEventProcessor.cs

@@ -23,7 +23,7 @@ internal ScreenshotEventProcessor(SentryUnityOptions sentryOptions, IApplication
 
     public SentryEvent? Process(SentryEvent @event, SentryHint hint)
     {
-        if (!MainThreadData.IsMainThread())
+        if (MainThreadData.IsMainThread() is not true)
         {
             _options.DiagnosticLogger?.LogDebug("Screenshot capture skipped. Can't capture screenshots on other than the main thread.");
             return @event;

src/Sentry.Unity/SentryUnitySDK.cs

@@ -28,8 +28,6 @@ private SentryUnitySdk(SentryUnityOptions options)
             return null;
         }
 
-        MainThreadData.CollectData();
-
         // On Standalone, we disable cache dir in case multiple app instances run over the same path.
         // Note: we cannot use a named Mutex, because Unit doesn't support it. Instead, we create a file with `FileShare.None`.
         // https://forum.unity.com/threads/unsupported-internal-call-for-il2cpp-mutex-createmutex_internal-named-mutexes-are-not-supported.387334/
@@ -49,6 +47,8 @@ private SentryUnitySdk(SentryUnityOptions options)
             }
         }
 
+        MainThreadData.CollectData();
+
         unitySdk._dotnetSdk = SentrySdk.Init(options);
 
         if (options.NativeContextWriter is { } contextWriter)

src/Sentry.Unity/UnityEventProcessor.cs

@@ -51,7 +51,7 @@ private void SetEventContext(IEventLike sentryEvent)
 
     private void PopulateDevice(Device device)
     {
-        if (!MainThreadData.IsMainThread())
+        if (MainThreadData.IsMainThread() is not true)
         {
             return;
         }
@@ -93,4 +93,14 @@ private void PopulateSdkIntegrations(SdkVersion sdkVersion)
 internal static class TagValueNormalizer
 {
     internal static string ToTagValue(this bool value) => value ? "true" : "false";
+
+    internal static string ToTagValue(this bool? value)
+    {
+        if (value.HasValue)
+        {
+            return value.Value ? "true" : "false";
+        }
+
+        return "unknown";
+    }
 }

src/Sentry.Unity/ViewHierarchyEventProcessor.cs

@@ -24,7 +24,7 @@ public ViewHierarchyEventProcessor(SentryUnityOptions sentryOptions)
 
     public SentryEvent? Process(SentryEvent @event, SentryHint hint)
     {
-        if (!MainThreadData.IsMainThread())
+        if (MainThreadData.IsMainThread() is not true)
         {
             _options.DiagnosticLogger?.LogDebug("Hierarchy capture skipped. Can't capture hierarchy on other than the main thread.");
             return @event;

test/Sentry.Unity.Tests/UnityEventScopeTests.cs

@@ -262,6 +262,7 @@ public void AppProtocol_Assigned()
         _testApplication = new TestApplication(productName: "TestGame");
         var sut = new UnityScopeUpdater(_sentryOptions, _testApplication);
         var scope = new Scope(_sentryOptions);
+        MainThreadData.CollectData();
 
         // act
         sut.ConfigureScope(scope);