From 51f668325855c6388a3998f9f5c72597272747dd Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Thu, 24 Nov 2022 17:16:43 +0100 Subject: [PATCH 1/9] closing mobile native --- package-dev/Runtime/SentryInitialization.cs | 9 +++++++++ src/Sentry.Unity.Android/SentryNativeAndroid.cs | 17 +++++++++++------ .../{SentryNative.cs => SentryNativeCocoa.cs} | 15 ++++++++++----- 3 files changed, 30 insertions(+), 11 deletions(-) rename src/Sentry.Unity.iOS/{SentryNative.cs => SentryNativeCocoa.cs} (86%) diff --git a/package-dev/Runtime/SentryInitialization.cs b/package-dev/Runtime/SentryInitialization.cs index 3f42071d1..4d3ffc74b 100644 --- a/package-dev/Runtime/SentryInitialization.cs +++ b/package-dev/Runtime/SentryInitialization.cs @@ -106,6 +106,15 @@ public static void Init() options.DiagnosticLogger?.LogDebug("Creating '{0}' span.", SubSystemSpanOperation); SubSystemRegistrationSpan = InitSpan.StartChild(SubSystemSpanOperation, "subsystem registration"); } +#endif + } + else + { + // Closing down the native layer that are set up during build and self-initialize +#if SENTRY_NATIVE_COCOA + SentryNativeCocoa.Close(options.DiagnosticLogger); +#elif SENTRY_NATIVE_ANDROID + SentryNativeAndroid.Close(options.DiagnosticLogger); #endif } } diff --git a/src/Sentry.Unity.Android/SentryNativeAndroid.cs b/src/Sentry.Unity.Android/SentryNativeAndroid.cs index 2924e8057..e42dc4f1c 100644 --- a/src/Sentry.Unity.Android/SentryNativeAndroid.cs +++ b/src/Sentry.Unity.Android/SentryNativeAndroid.cs @@ -53,15 +53,20 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry "Failed to reinstall backend. Captured native crashes will miss scope data and tag.", e); } - ApplicationAdapter.Instance.Quitting += () => - { - // Sentry Native is initialized and closed by the Java SDK, no need to call into it directly - options.DiagnosticLogger?.LogDebug("Closing the sentry-java SDK"); - SentryJava.Close(); - }; + ApplicationAdapter.Instance.Quitting += () => Close(options.DiagnosticLogger); options.DefaultUserId = SentryJava.GetInstallationId(); } } + + /// + /// Closes the native Android support. + /// + public static void Close(IDiagnosticLogger? logger = null) + { + // Sentry Native is initialized and closed by the Java SDK, no need to call into it directly + logger?.LogDebug("Closing the sentry-java SDK"); + SentryJava.Close(); + } } } diff --git a/src/Sentry.Unity.iOS/SentryNative.cs b/src/Sentry.Unity.iOS/SentryNativeCocoa.cs similarity index 86% rename from src/Sentry.Unity.iOS/SentryNative.cs rename to src/Sentry.Unity.iOS/SentryNativeCocoa.cs index a98044cb0..707ad4792 100644 --- a/src/Sentry.Unity.iOS/SentryNative.cs +++ b/src/Sentry.Unity.iOS/SentryNativeCocoa.cs @@ -55,15 +55,20 @@ internal static void Configure(SentryUnityOptions options, ISentryUnityInfo sent return crashedLastRun; }; - ApplicationAdapter.Instance.Quitting += () => - { - options.DiagnosticLogger?.LogDebug("Closing the sentry-cocoa SDK"); - SentryCocoaBridgeProxy.Close(); - }; + ApplicationAdapter.Instance.Quitting += () => Close(options.DiagnosticLogger); if (sentryUnityInfo.IL2CPP) { options.DefaultUserId = SentryCocoaBridgeProxy.GetInstallationId(); } } + + /// + /// Closes the native Android support. + /// + public static void Close(IDiagnosticLogger? logger = null) + { + logger?.LogDebug("Closing the sentry-cocoa SDK"); + SentryCocoaBridgeProxy.Close(); + } } } From df44ec94f12cc52309a3d2a939ac40f179c45040 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Thu, 24 Nov 2022 17:20:40 +0100 Subject: [PATCH 2/9] fixed comment --- src/Sentry.Unity.iOS/SentryNativeCocoa.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sentry.Unity.iOS/SentryNativeCocoa.cs b/src/Sentry.Unity.iOS/SentryNativeCocoa.cs index 707ad4792..fad7dd42e 100644 --- a/src/Sentry.Unity.iOS/SentryNativeCocoa.cs +++ b/src/Sentry.Unity.iOS/SentryNativeCocoa.cs @@ -63,7 +63,7 @@ internal static void Configure(SentryUnityOptions options, ISentryUnityInfo sent } /// - /// Closes the native Android support. + /// Closes the native Cocoa support. /// public static void Close(IDiagnosticLogger? logger = null) { From f8b6c55dc258699f37070aebb12a89f7bb81e301 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Thu, 24 Nov 2022 17:29:10 +0100 Subject: [PATCH 3/9] Updated CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c933c3501..739598b2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixes + +- During the self init, if the SDK has been disabled, the native layer on mobile now closes down too ([#1092](https://github.com/getsentry/sentry-unity/pull/1092)) + ### Dependencies - Bump Cocoa SDK from v7.30.2 to v7.31.2 ([#1079](https://github.com/getsentry/sentry-unity/pull/1079), [#1082](https://github.com/getsentry/sentry-unity/pull/1082), [#1089](https://github.com/getsentry/sentry-unity/pull/1089)) From e9c6ae2e28a46dd7e56803694b5646833d1eef90 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Thu, 24 Nov 2022 19:40:01 +0100 Subject: [PATCH 4/9] expected symbol count --- test/Scripts.Integration.Test/common.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Scripts.Integration.Test/common.ps1 b/test/Scripts.Integration.Test/common.ps1 index 5fc42b226..a1bf9e098 100644 --- a/test/Scripts.Integration.Test/common.ps1 +++ b/test/Scripts.Integration.Test/common.ps1 @@ -289,8 +289,8 @@ function CheckSymbolServerOutput([string] $buildMethod, [string] $symbolServerOu If ($unity2020OrHigher) { $expectedFiles = @( - "libil2cpp.sym.so: count=$($withSources ? 3 : 2)", - "libil2cpp.dbg.so: count=$($withSources ? 2 : 1)" + "libil2cpp.sym.so: count=$($withSources ? 2 : 1)", + "libil2cpp.dbg.so: count=$($withSources ? 3 : 2)" ) + $expectedFiles } else From 638459669efcc6a50de9e5da5c56f361dfa30ce3 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Mon, 28 Nov 2022 15:31:26 +0100 Subject: [PATCH 5/9] closing native callback --- package-dev/Runtime/SentryInitialization.cs | 6 +- .../SentryNativeAndroid.cs | 2 + src/Sentry.Unity.iOS/SentryNativeCocoa.cs | 2 + .../ScriptableSentryUnityOptions.cs | 2 +- src/Sentry.Unity/SentryUnity.cs | 77 +++++++++++-------- src/Sentry.Unity/SentryUnityOptions.cs | 6 ++ 6 files changed, 59 insertions(+), 36 deletions(-) diff --git a/package-dev/Runtime/SentryInitialization.cs b/package-dev/Runtime/SentryInitialization.cs index 4d3ffc74b..c5508ec88 100644 --- a/package-dev/Runtime/SentryInitialization.cs +++ b/package-dev/Runtime/SentryInitialization.cs @@ -85,7 +85,7 @@ public static void Init() nativeInitException = new Exception("Sentry native error capture configuration failed.", e); } - SentryUnity.Init(options); + var unitySDK = SentryUnity.Init(options); if (nativeInitException != null) { SentrySdk.CaptureException(nativeInitException); @@ -112,9 +112,9 @@ public static void Init() { // Closing down the native layer that are set up during build and self-initialize #if SENTRY_NATIVE_COCOA - SentryNativeCocoa.Close(options.DiagnosticLogger); + SentryNativeCocoa.Close(options.DiagnosticLogger); #elif SENTRY_NATIVE_ANDROID - SentryNativeAndroid.Close(options.DiagnosticLogger); + SentryNativeAndroid.Close(options.DiagnosticLogger); #endif } } diff --git a/src/Sentry.Unity.Android/SentryNativeAndroid.cs b/src/Sentry.Unity.Android/SentryNativeAndroid.cs index e42dc4f1c..9d23ee545 100644 --- a/src/Sentry.Unity.Android/SentryNativeAndroid.cs +++ b/src/Sentry.Unity.Android/SentryNativeAndroid.cs @@ -1,6 +1,7 @@ using System; using Sentry.Extensibility; using Sentry.Unity.Integrations; +using Sentry.Unity.NativeUtils; namespace Sentry.Unity.Android { @@ -54,6 +55,7 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry } ApplicationAdapter.Instance.Quitting += () => Close(options.DiagnosticLogger); + options.NativeSupportCloseCallback = () => Close(options.DiagnosticLogger); options.DefaultUserId = SentryJava.GetInstallationId(); } diff --git a/src/Sentry.Unity.iOS/SentryNativeCocoa.cs b/src/Sentry.Unity.iOS/SentryNativeCocoa.cs index fad7dd42e..d4ff7c507 100644 --- a/src/Sentry.Unity.iOS/SentryNativeCocoa.cs +++ b/src/Sentry.Unity.iOS/SentryNativeCocoa.cs @@ -56,6 +56,8 @@ internal static void Configure(SentryUnityOptions options, ISentryUnityInfo sent return crashedLastRun; }; ApplicationAdapter.Instance.Quitting += () => Close(options.DiagnosticLogger); + options.NativeSupportCloseCallback += () => Close(options.DiagnosticLogger); + if (sentryUnityInfo.IL2CPP) { options.DefaultUserId = SentryCocoaBridgeProxy.GetInstallationId(); diff --git a/src/Sentry.Unity/ScriptableSentryUnityOptions.cs b/src/Sentry.Unity/ScriptableSentryUnityOptions.cs index cfbe57379..38c272f87 100644 --- a/src/Sentry.Unity/ScriptableSentryUnityOptions.cs +++ b/src/Sentry.Unity/ScriptableSentryUnityOptions.cs @@ -77,7 +77,7 @@ public static string GetConfigPath(string? notDefaultConfigName = null) [field: SerializeField] public Sentry.Unity.ScriptableOptionsConfiguration? OptionsConfiguration { get; set; } - /// Actual type is `Sentry.Unity.Editor.ScriptableOptionsConfiguration` but we can't reference it here because we don't depend on the editor Assembly. + // Actual type is `Sentry.Unity.Editor.ScriptableOptionsConfiguration` but we can't reference it here because we don't depend on the editor Assembly. [field: SerializeField] public ScriptableObject? BuildtimeOptionsConfiguration { get; set; } [field: SerializeField] public bool Debug { get; set; } = true; diff --git a/src/Sentry.Unity/SentryUnity.cs b/src/Sentry.Unity/SentryUnity.cs index f2663339a..62e48c82c 100644 --- a/src/Sentry.Unity/SentryUnity.cs +++ b/src/Sentry.Unity/SentryUnity.cs @@ -1,7 +1,9 @@ using System; using System.IO; using System.ComponentModel; +using System.Diagnostics.Tracing; using System.Threading.Tasks; +using System.Xml; using Sentry.Extensibility; using Sentry.Unity.Integrations; using UnityEngine; @@ -13,6 +15,9 @@ namespace Sentry.Unity /// public static class SentryUnity { + private static IDisposable? DotnetSdk; + private static SentryUnityOptions? Options; + private static FileStream? LockFile; /// @@ -33,39 +38,41 @@ public static void Init(Action sentryUnityOptionsConfigure) [EditorBrowsable(EditorBrowsableState.Never)] public static void Init(SentryUnityOptions options) { - options.SetupLogging(); - if (options.ShouldInitializeSdk()) + Options = options; + + Options.SetupLogging(); + if (Options.ShouldInitializeSdk()) { // 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/ - if (ApplicationAdapter.Instance.Platform is RuntimePlatform.WindowsPlayer && options.CacheDirectoryPath is not null) + if (ApplicationAdapter.Instance.Platform is RuntimePlatform.WindowsPlayer && Options.CacheDirectoryPath is not null) { try { - LockFile = new FileStream(Path.Combine(options.CacheDirectoryPath, "sentry-unity.lock"), FileMode.OpenOrCreate, + LockFile = new FileStream(Path.Combine(Options.CacheDirectoryPath, "sentry-unity.lock"), FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None); } catch (Exception ex) { - options.DiagnosticLogger?.LogWarning("An exception was thrown while trying to " + - "acquire a lockfile on the config directory: .NET event cache will be disabled.", ex); - options.CacheDirectoryPath = null; - options.AutoSessionTracking = false; + Options.DiagnosticLogger?.LogWarning("An exception was thrown while trying to " + + "acquire a lockfile on the config directory: .NET event cache will be disabled.", ex); + Options.CacheDirectoryPath = null; + Options.AutoSessionTracking = false; } } - var sentryDotNet = SentrySdk.Init(options); + DotnetSdk = SentrySdk.Init(options); - if (options.AttachScreenshot) + if (Options.AttachScreenshot) { SentrySdk.ConfigureScope(s => s.AddAttachment(new ScreenshotAttachment( new ScreenshotAttachmentContent(options, SentryMonoBehaviour.Instance)))); } - if (options.NativeContextWriter is { } contextWriter) + if (Options.NativeContextWriter is { } contextWriter) { SentrySdk.ConfigureScope((scope) => { @@ -73,35 +80,41 @@ public static void Init(SentryUnityOptions options) { if (t.Exception is not null) { - options.DiagnosticLogger?.LogWarning( + Options.DiagnosticLogger?.LogWarning( "Failed to synchronize scope to the native SDK: {0}", t.Exception); } }); }); } + ApplicationAdapter.Instance.Quitting += Close; + } + } - ApplicationAdapter.Instance.Quitting += () => + /// + /// Closes the Sentry Unity SDK + /// + [EditorBrowsable(EditorBrowsableState.Never)] + public static void Close() + { + Options?.DiagnosticLogger?.LogDebug("Closing the sentry-dotnet SDK"); + try + { + Options?.NativeSupportCloseCallback?.Invoke(); + DotnetSdk?.Dispose(); + } + finally + { + try { - options.DiagnosticLogger?.LogDebug("Closing the sentry-dotnet SDK"); - try - { - sentryDotNet.Dispose(); - } - finally - { - try - { - // We don't really need to close, Windows would release the lock anyway, but let's be nice. - LockFile?.Close(); - } - catch (Exception ex) - { - options.DiagnosticLogger?.Log(SentryLevel.Warning, - "Exception while releasing the lockfile on the config directory.", ex); - } - } - }; + // We don't really need to close, Windows would release the lock anyway, but let's be nice. + LockFile?.Close(); + } + catch (Exception ex) + { + Options?.DiagnosticLogger?.Log(SentryLevel.Warning, + "Exception while releasing the lockfile on the config directory.", ex); + } } } } diff --git a/src/Sentry.Unity/SentryUnityOptions.cs b/src/Sentry.Unity/SentryUnityOptions.cs index f7752a293..f68a2819e 100644 --- a/src/Sentry.Unity/SentryUnityOptions.cs +++ b/src/Sentry.Unity/SentryUnityOptions.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Linq; using System.Text.RegularExpressions; @@ -157,6 +158,11 @@ internal string? DefaultUserId /// internal ContextWriter? NativeContextWriter { get; set; } = null; + /// + /// Used to close down the native SDK + /// + internal Action? NativeSupportCloseCallback { get; set; } = null; + internal List SdkIntegrationNames { get; set; } = new(); public SentryUnityOptions() : this(false, null, ApplicationAdapter.Instance) { } From 1fb60311e1ec5d70c5e092b4728abc480d74e87f Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Mon, 28 Nov 2022 16:01:37 +0100 Subject: [PATCH 6/9] closing cleanup --- package-dev/Runtime/SentryInitialization.cs | 2 +- src/Sentry.Unity.Android/SentryNativeAndroid.cs | 2 -- src/Sentry.Unity.iOS/SentryNativeCocoa.cs | 3 +-- src/Sentry.Unity/SentryUnity.cs | 10 ++++++++-- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/package-dev/Runtime/SentryInitialization.cs b/package-dev/Runtime/SentryInitialization.cs index c5508ec88..30607c764 100644 --- a/package-dev/Runtime/SentryInitialization.cs +++ b/package-dev/Runtime/SentryInitialization.cs @@ -85,7 +85,7 @@ public static void Init() nativeInitException = new Exception("Sentry native error capture configuration failed.", e); } - var unitySDK = SentryUnity.Init(options); + SentryUnity.Init(options); if (nativeInitException != null) { SentrySdk.CaptureException(nativeInitException); diff --git a/src/Sentry.Unity.Android/SentryNativeAndroid.cs b/src/Sentry.Unity.Android/SentryNativeAndroid.cs index 9d23ee545..f7fc898cc 100644 --- a/src/Sentry.Unity.Android/SentryNativeAndroid.cs +++ b/src/Sentry.Unity.Android/SentryNativeAndroid.cs @@ -54,9 +54,7 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry "Failed to reinstall backend. Captured native crashes will miss scope data and tag.", e); } - ApplicationAdapter.Instance.Quitting += () => Close(options.DiagnosticLogger); options.NativeSupportCloseCallback = () => Close(options.DiagnosticLogger); - options.DefaultUserId = SentryJava.GetInstallationId(); } } diff --git a/src/Sentry.Unity.iOS/SentryNativeCocoa.cs b/src/Sentry.Unity.iOS/SentryNativeCocoa.cs index d4ff7c507..8b8fb31fa 100644 --- a/src/Sentry.Unity.iOS/SentryNativeCocoa.cs +++ b/src/Sentry.Unity.iOS/SentryNativeCocoa.cs @@ -55,9 +55,8 @@ internal static void Configure(SentryUnityOptions options, ISentryUnityInfo sent return crashedLastRun; }; - ApplicationAdapter.Instance.Quitting += () => Close(options.DiagnosticLogger); - options.NativeSupportCloseCallback += () => Close(options.DiagnosticLogger); + options.NativeSupportCloseCallback += () => Close(options.DiagnosticLogger); if (sentryUnityInfo.IL2CPP) { options.DefaultUserId = SentryCocoaBridgeProxy.GetInstallationId(); diff --git a/src/Sentry.Unity/SentryUnity.cs b/src/Sentry.Unity/SentryUnity.cs index 62e48c82c..f06cbff84 100644 --- a/src/Sentry.Unity/SentryUnity.cs +++ b/src/Sentry.Unity/SentryUnity.cs @@ -59,7 +59,6 @@ public static void Init(SentryUnityOptions options) "acquire a lockfile on the config directory: .NET event cache will be disabled.", ex); Options.CacheDirectoryPath = null; Options.AutoSessionTracking = false; - } } @@ -100,7 +99,14 @@ public static void Close() Options?.DiagnosticLogger?.LogDebug("Closing the sentry-dotnet SDK"); try { - Options?.NativeSupportCloseCallback?.Invoke(); + ApplicationAdapter.Instance.Quitting -= Close; + + if (Options is not null) + { + Options.NativeSupportCloseCallback?.Invoke(); + Options.NativeSupportCloseCallback = null; + } + DotnetSdk?.Dispose(); } finally From 5ee9f5526ba154a5e9f28b5828536eb3f7b5c82d Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Mon, 28 Nov 2022 16:04:20 +0100 Subject: [PATCH 7/9] Updated CHANGELOG.md --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 739598b2b..707ff3879 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,9 @@ ## Unreleased -### Fixes +### Features -- During the self init, if the SDK has been disabled, the native layer on mobile now closes down too ([#1092](https://github.com/getsentry/sentry-unity/pull/1092)) +- Added missing SDK closing functionality ([#1092](https://github.com/getsentry/sentry-unity/pull/1092)) ### Dependencies From 96e285d153c58ba21717243c0c70d0887a15a2b0 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Mon, 28 Nov 2022 16:07:06 +0100 Subject: [PATCH 8/9] Updated CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3505adf8..a27bb7700 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Features - Added missing SDK closing functionality ([#1092](https://github.com/getsentry/sentry-unity/pull/1092)) + ### Fixes - Fixed logging for automated debug symbol upload for iOS ([#1091](https://github.com/getsentry/sentry-unity/pull/1091)) From b0e8f45c11e1b71613015914606a2fc7725aa4b3 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Mon, 5 Dec 2022 11:53:11 +0100 Subject: [PATCH 9/9] review --- src/Sentry.Unity/SentryUnity.cs | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/Sentry.Unity/SentryUnity.cs b/src/Sentry.Unity/SentryUnity.cs index f06cbff84..3f31a4b4a 100644 --- a/src/Sentry.Unity/SentryUnity.cs +++ b/src/Sentry.Unity/SentryUnity.cs @@ -108,19 +108,23 @@ public static void Close() } DotnetSdk?.Dispose(); + DotnetSdk = null; } - finally + catch (Exception ex) { - try - { - // We don't really need to close, Windows would release the lock anyway, but let's be nice. - LockFile?.Close(); - } - catch (Exception ex) - { - Options?.DiagnosticLogger?.Log(SentryLevel.Warning, - "Exception while releasing the lockfile on the config directory.", ex); - } + Options?.DiagnosticLogger?.Log(SentryLevel.Warning, + "Exception while closing the .NET SDK.", ex); + } + + try + { + // We don't really need to close, Windows would release the lock anyway, but let's be nice. + LockFile?.Close(); + } + catch (Exception ex) + { + Options?.DiagnosticLogger?.Log(SentryLevel.Warning, + "Exception while releasing the lockfile on the config directory.", ex); } } }