From 881d807c888bf42f8464e04b6d39b003a5900042 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 7 Mar 2022 16:27:09 +0100 Subject: [PATCH 1/9] feat: native CrashedLastRun() --- src/Sentry.Unity.Native/SentryNative.cs | 35 ++++++------ src/Sentry.Unity.Native/SentryNativeBridge.cs | 56 ++++++++++++++----- 2 files changed, 61 insertions(+), 30 deletions(-) diff --git a/src/Sentry.Unity.Native/SentryNative.cs b/src/Sentry.Unity.Native/SentryNative.cs index a5005a404..efae0619d 100644 --- a/src/Sentry.Unity.Native/SentryNative.cs +++ b/src/Sentry.Unity.Native/SentryNative.cs @@ -1,5 +1,6 @@ using Sentry.Extensibility; using Sentry.Unity.Integrations; +using System.Collections.Generic; namespace Sentry.Unity.Native { @@ -8,6 +9,8 @@ namespace Sentry.Unity.Native /// public static class SentryNative { + private static Dictionary perDirectoryCrashInfo = new Dictionary(); + /// /// Configures the native SDK. /// @@ -24,24 +27,22 @@ public static void Configure(SentryUnityOptions options) }; options.ScopeObserver = new NativeScopeObserver(options); options.EnableScopeSync = true; - // options.CrashedLastRun = () => - // { - // var crashedLastRun = SentryJava.CrashedLastRun(); - // if (crashedLastRun is null) - // { - // // Could happen if the Android SDK wasn't initialized before the .NET layer. - // options.DiagnosticLogger? - // .LogWarning("Unclear from the native SDK if the previous run was a crash. Assuming it was not."); - // crashedLastRun = false; - // } - // else - // { - // options.DiagnosticLogger? - // .LogDebug("Native Android SDK reported: 'crashedLastRun': '{0}'", crashedLastRun); - // } - // return crashedLastRun.Value; - // }; + // Note: we must actually call the function now and on every other call use the value we get here. + // Additionally, we cannot call this multiple times for the same directory, because the result changes + // on subsequent runs. Therefore, we cache the value during the whole runtime of the application. + var cacheDirectory = SentryNativeBridge.GetCacheDirectory(options); + bool crashedLastRun = false; + lock (perDirectoryCrashInfo) + { + if (!perDirectoryCrashInfo.TryGetValue(cacheDirectory, out crashedLastRun)) + { + crashedLastRun = SentryNativeBridge.HandleCrashedLastRun(options, cacheDirectory); + perDirectoryCrashInfo.Add(cacheDirectory, crashedLastRun); + } + } + options.CrashedLastRun = () => crashedLastRun; + // At this point Unity has taken the signal handler and will not invoke the original handler (Sentry) // So we register our backend once more to make sure user-defined data is available in the crash report. SentryNativeBridge.ReinstallBackend(); diff --git a/src/Sentry.Unity.Native/SentryNativeBridge.cs b/src/Sentry.Unity.Native/SentryNativeBridge.cs index 37b0988bb..1b4429a33 100644 --- a/src/Sentry.Unity.Native/SentryNativeBridge.cs +++ b/src/Sentry.Unity.Native/SentryNativeBridge.cs @@ -3,7 +3,6 @@ using System.Runtime.InteropServices; using Sentry.Extensibility; using AOT; -using System.Threading; using System.Text; namespace Sentry.Unity @@ -52,19 +51,16 @@ public static void Init(SentryUnityOptions options) options.DiagnosticLogger?.LogDebug("Disabling native auto session tracking"); sentry_options_set_auto_session_tracking(cOptions, 0); - if (options.CacheDirectoryPath is not null) + var dir = GetCacheDirectory(options); + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { - var dir = Path.Combine(options.CacheDirectoryPath, "SentryNative"); - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) - { - options.DiagnosticLogger?.LogDebug("Setting CacheDirectoryPath on Windows: {0}", dir); - sentry_options_set_database_pathw(cOptions, dir); - } - else - { - options.DiagnosticLogger?.LogDebug("Setting CacheDirectoryPath: {0}", dir); - sentry_options_set_database_path(cOptions, dir); - } + options.DiagnosticLogger?.LogDebug("Setting CacheDirectoryPath on Windows: {0}", dir); + sentry_options_set_database_pathw(cOptions, dir); + } + else + { + options.DiagnosticLogger?.LogDebug("Setting CacheDirectoryPath: {0}", dir); + sentry_options_set_database_path(cOptions, dir); } if (options.DiagnosticLogger is null) @@ -83,6 +79,40 @@ public static void Init(SentryUnityOptions options) } public static void Close() => sentry_close(); + + /// Call after native init() to check if the application has crashed in the previous run and clear the status. + /// Because the file is removed, the result will change on subsequent calls so it must be cached for the current runtime. + internal static bool HandleCrashedLastRun(SentryUnityOptions options, string cacheDirectory) + { + // See the file name in [sentry__write_crash_marker](https://github.com/getsentry/sentry-native/blob/0.4.12/src/sentry_database.c#L239) + var crashFile = Path.Combine(cacheDirectory, "last_crash"); + + if (File.Exists(crashFile)) + { + options.DiagnosticLogger?.LogWarning("The last session seems to have crashed on {0}.", File.ReadAllText(crashFile)); + + // Remove the file so that it's not there next time, unless there's another crash. + File.Delete(crashFile); + + return true; + } + + options.DiagnosticLogger?.LogDebug("The last session looks crash-free - file {0} doesn't exist.", crashFile); + return false; + } + + internal static string GetCacheDirectory(SentryUnityOptions options) + { + if (options.CacheDirectoryPath is null) + { + // same as the default of sentry-native + return Path.Combine(Directory.GetCurrentDirectory(), ".sentry-native"); + } + else + { + return Path.Combine(options.CacheDirectoryPath, "SentryNative"); + } + } // libsentry.so [DllImport("sentry")] From 5fd8bbd671b60242db3b650896c412134ec880ad Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 7 Mar 2022 16:32:05 +0100 Subject: [PATCH 2/9] test: CrashedLastRun() --- .../Assets/Scripts/SmokeTester.cs | 19 ++++++++++++++++++- .../integration-run-smoke-test.ps1 | 5 ++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/samples/unity-of-bugs/Assets/Scripts/SmokeTester.cs b/samples/unity-of-bugs/Assets/Scripts/SmokeTester.cs index 74a98dbea..e46cbc2e2 100644 --- a/samples/unity-of-bugs/Assets/Scripts/SmokeTester.cs +++ b/samples/unity-of-bugs/Assets/Scripts/SmokeTester.cs @@ -82,6 +82,10 @@ public void Start() { SmokeTestCrash(); } + else if (arg == "post-crash") + { + PostCrashTest(); + } else { Debug.Log($"Unknown command line argument: {arg}"); @@ -127,7 +131,10 @@ public static void SmokeTest() { Debug.Log("SMOKE TEST: Start"); - InitSentry(new SentryUnityOptions() { CreateHttpClientHandler = () => t }); + var options = new SentryUnityOptions() { CreateHttpClientHandler = () => t }; + InitSentry(options); + + t.Expect("options.CrashedLastRun() == false", !options.CrashedLastRun()); var currentMessage = 0; t.ExpectMessage(currentMessage, "'type':'session'"); @@ -196,6 +203,16 @@ public static void SmokeTestCrash() Application.Quit(-1); } + public static void PostCrashTest() + { + var options = new SentryUnityOptions(); + InitSentry(options); + + var t = new TestHandler(); + t.Expect("options.CrashedLastRun() == true", options.CrashedLastRun()); + t.Pass(); + } + private static void AddContext() { SentrySdk.AddBreadcrumb("crumb", "bread", "error", new Dictionary() { { "foo", "bar" } }, BreadcrumbLevel.Critical); diff --git a/test/Scripts.Integration.Test/integration-run-smoke-test.ps1 b/test/Scripts.Integration.Test/integration-run-smoke-test.ps1 index def8d51e9..8dabe8653 100644 --- a/test/Scripts.Integration.Test/integration-run-smoke-test.ps1 +++ b/test/Scripts.Integration.Test/integration-run-smoke-test.ps1 @@ -85,9 +85,6 @@ function RunTest([string] $type) { Else { $info = "Test process finished with status code $($process.ExitCode)." If ($type -ne "smoke-crash") { - if ("$AppDataDir" -ne "") { - Get-Content "$AppDataDir/Player.log" - } throw $info } Write-Host $info @@ -170,5 +167,7 @@ if ($Crash) { else { Write-Error "smoke-crash test $run/$runs : FAILED" } + + RunTest "post-crash" } } From 565aff686cdb5e175953225b7a00f1b480ee49c4 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 9 Mar 2022 16:01:58 +0100 Subject: [PATCH 3/9] fix: iOS CrashedLastRun() --- package-dev/Plugins/iOS/SentryNativeBridge.m | 4 ++-- src/Sentry.Unity.iOS/SentryCocoaBridgeProxy.cs | 2 +- src/Sentry.Unity.iOS/SentryNativeIos.cs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package-dev/Plugins/iOS/SentryNativeBridge.m b/package-dev/Plugins/iOS/SentryNativeBridge.m index 8c99f796e..4de55d26e 100644 --- a/package-dev/Plugins/iOS/SentryNativeBridge.m +++ b/package-dev/Plugins/iOS/SentryNativeBridge.m @@ -2,8 +2,8 @@ NS_ASSUME_NONNULL_BEGIN -bool CrashedLastRun() { - return[SentrySDK crashedLastRun]; +int CrashedLastRun() { + return [SentrySDK crashedLastRun] ? 1 : 0; } void Close() { diff --git a/src/Sentry.Unity.iOS/SentryCocoaBridgeProxy.cs b/src/Sentry.Unity.iOS/SentryCocoaBridgeProxy.cs index 45cee2766..0bc68e37a 100644 --- a/src/Sentry.Unity.iOS/SentryCocoaBridgeProxy.cs +++ b/src/Sentry.Unity.iOS/SentryCocoaBridgeProxy.cs @@ -12,7 +12,7 @@ namespace Sentry.Unity.iOS internal static class SentryCocoaBridgeProxy { [DllImport("__Internal")] - public static extern bool CrashedLastRun(); + public static extern int CrashedLastRun(); [DllImport("__Internal")] public static extern void Close(); diff --git a/src/Sentry.Unity.iOS/SentryNativeIos.cs b/src/Sentry.Unity.iOS/SentryNativeIos.cs index 32d19075f..e9780262d 100644 --- a/src/Sentry.Unity.iOS/SentryNativeIos.cs +++ b/src/Sentry.Unity.iOS/SentryNativeIos.cs @@ -20,7 +20,7 @@ public static void Configure(SentryUnityOptions options) options.EnableScopeSync = true; options.CrashedLastRun = () => { - var crashedLastRun = SentryCocoaBridgeProxy.CrashedLastRun(); + var crashedLastRun = SentryCocoaBridgeProxy.CrashedLastRun() == 1; options.DiagnosticLogger? .LogDebug("Native iOS SDK reported: 'crashedLastRun': '{0}'", crashedLastRun); From d0877dbf11bf4780cbff45def773c7d4bc095854 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 9 Mar 2022 16:28:34 +0100 Subject: [PATCH 4/9] update sentry-native to the latest HEAD --- modules/sentry-native | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/sentry-native b/modules/sentry-native index 179ee2b14..9eecb1bdf 160000 --- a/modules/sentry-native +++ b/modules/sentry-native @@ -1 +1 @@ -Subproject commit 179ee2b14c79a786fd1e75a696a05f95a8b59e77 +Subproject commit 9eecb1bdf2c7762ba7babe9ea437545a1f759bdc From 62d33de3baa97c918834e93f62554114e7475f58 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 9 Mar 2022 16:45:02 +0100 Subject: [PATCH 5/9] refactor: use native crashed_last_run() --- src/Sentry.Unity.Native/SentryNative.cs | 6 +++- src/Sentry.Unity.Native/SentryNativeBridge.cs | 31 +++++++++---------- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/Sentry.Unity.Native/SentryNative.cs b/src/Sentry.Unity.Native/SentryNative.cs index efae0619d..85f8391af 100644 --- a/src/Sentry.Unity.Native/SentryNative.cs +++ b/src/Sentry.Unity.Native/SentryNative.cs @@ -1,6 +1,7 @@ using Sentry.Extensibility; using Sentry.Unity.Integrations; using System.Collections.Generic; +using Sentry.Extensibility; namespace Sentry.Unity.Native { @@ -37,8 +38,11 @@ public static void Configure(SentryUnityOptions options) { if (!perDirectoryCrashInfo.TryGetValue(cacheDirectory, out crashedLastRun)) { - crashedLastRun = SentryNativeBridge.HandleCrashedLastRun(options, cacheDirectory); + crashedLastRun = SentryNativeBridge.HandleCrashedLastRun(options); perDirectoryCrashInfo.Add(cacheDirectory, crashedLastRun); + + options.DiagnosticLogger? + .LogDebug("Native SDK reported: 'crashedLastRun': '{0}'", crashedLastRun); } } options.CrashedLastRun = () => crashedLastRun; diff --git a/src/Sentry.Unity.Native/SentryNativeBridge.cs b/src/Sentry.Unity.Native/SentryNativeBridge.cs index 1b4429a33..c9e862cd1 100644 --- a/src/Sentry.Unity.Native/SentryNativeBridge.cs +++ b/src/Sentry.Unity.Native/SentryNativeBridge.cs @@ -19,6 +19,9 @@ namespace Sentry.Unity /// public static class SentryNativeBridge { + + public static bool CrashedLastRun; + public static void Init(SentryUnityOptions options) { var cOptions = sentry_options_new(); @@ -79,26 +82,14 @@ public static void Init(SentryUnityOptions options) } public static void Close() => sentry_close(); - + /// Call after native init() to check if the application has crashed in the previous run and clear the status. /// Because the file is removed, the result will change on subsequent calls so it must be cached for the current runtime. - internal static bool HandleCrashedLastRun(SentryUnityOptions options, string cacheDirectory) + internal static bool HandleCrashedLastRun(SentryUnityOptions options) { - // See the file name in [sentry__write_crash_marker](https://github.com/getsentry/sentry-native/blob/0.4.12/src/sentry_database.c#L239) - var crashFile = Path.Combine(cacheDirectory, "last_crash"); - - if (File.Exists(crashFile)) - { - options.DiagnosticLogger?.LogWarning("The last session seems to have crashed on {0}.", File.ReadAllText(crashFile)); - - // Remove the file so that it's not there next time, unless there's another crash. - File.Delete(crashFile); - - return true; - } - - options.DiagnosticLogger?.LogDebug("The last session looks crash-free - file {0} doesn't exist.", crashFile); - return false; + var result = sentry_get_crashed_last_run() == 1; + sentry_clear_crashed_last_run(); + return result; } internal static string GetCacheDirectory(SentryUnityOptions options) @@ -202,6 +193,12 @@ private static void nativeLog(int cLevel, string message, IntPtr args, IntPtr us [DllImport("sentry")] private static extern int sentry_close(); + [DllImport("sentry")] + private static extern int sentry_get_crashed_last_run(); + + [DllImport("sentry")] + private static extern int sentry_clear_crashed_last_run(); + /// /// Re-installs the sentry-native backend essentially retaking the signal handlers. /// From fc0152a589ddc9d86aa07a76556e7d6aad1597bb Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 9 Mar 2022 18:04:53 +0100 Subject: [PATCH 6/9] chore: update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec5a879a6..98da1ecff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,11 @@ - iOS builds no longer break when native support disabled or not available ([#592](https://github.com/getsentry/sentry-unity/pull/592)) - Close sentry instance when quitting the app ([#608](https://github.com/getsentry/sentry-unity/pull/608)) +- iOS options.CrashedLastRun() reported an incorrect value ([#615](https://github.com/getsentry/sentry-unity/pull/615)) ### Features +- Update native SDK to v0.4.16 (pre-release) ([#615](https://github.com/getsentry/sentry-unity/pull/615)) - Windows native support (64-bit) - native crash handler ([#380](https://github.com/getsentry/sentry-unity/pull/380)) - configuration & log forwarding ([#577](https://github.com/getsentry/sentry-unity/pull/577)) From fdded8555134cd12a406913904b0c5f1e0979148 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 10 Mar 2022 09:59:43 +0100 Subject: [PATCH 7/9] ci: simple retry logic for the smoke-crash test --- .../integration-run-smoke-test.ps1 | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/test/Scripts.Integration.Test/integration-run-smoke-test.ps1 b/test/Scripts.Integration.Test/integration-run-smoke-test.ps1 index 8dabe8653..5254a3f8c 100644 --- a/test/Scripts.Integration.Test/integration-run-smoke-test.ps1 +++ b/test/Scripts.Integration.Test/integration-run-smoke-test.ps1 @@ -122,7 +122,8 @@ if ($Smoke) { # Native crash test if ($Crash) { - $runs = 1 # You can increase this to run the crash test multiple times in a loop (until it fails) + # You can increase this to retry multiple times. Seems a bit flaky at the moment in CI. + $runs = 5 for ($run = 1; $run -le $runs; $run++) { $httpServer = RunApiServer RunTest "smoke-crash" @@ -131,8 +132,8 @@ if ($Crash) { $successMessage = "POST /api/12345/minidump/" Write-Host "Waiting for the expected message to appear in the server output logs ..." - # Wait for 1 minute (600 * 100 milliseconds) until the expected message comes in - for ($i = 0; $i -lt 600; $i++) { + # Wait for 30 seconds (300 * 100 milliseconds) until the expected message comes in + for ($i = 0; $i -lt 300; $i++) { $output = (Get-Content $httpServer.outFile -Raw) + (Get-Content $httpServer.errFile -Raw) if ("$output".Contains($successMessage)) { break @@ -164,10 +165,10 @@ if ($Crash) { if ($output.Contains($successMessage)) { Write-Host "smoke-crash test $run/$runs : PASSED" -ForegroundColor Green } - else { + elseif ($run -ne $runs) { Write-Error "smoke-crash test $run/$runs : FAILED" } - - RunTest "post-crash" } + + RunTest "post-crash" } From 368c47dc4cec12bfbe2494cef2f57741e939c1d5 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Thu, 10 Mar 2022 12:10:09 +0100 Subject: [PATCH 8/9] fix: merge-related import duplication --- src/Sentry.Unity.Native/SentryNative.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Sentry.Unity.Native/SentryNative.cs b/src/Sentry.Unity.Native/SentryNative.cs index 85f8391af..d635a04c8 100644 --- a/src/Sentry.Unity.Native/SentryNative.cs +++ b/src/Sentry.Unity.Native/SentryNative.cs @@ -1,7 +1,6 @@ using Sentry.Extensibility; using Sentry.Unity.Integrations; using System.Collections.Generic; -using Sentry.Extensibility; namespace Sentry.Unity.Native { From 9490c0c76135cdf43574036f0c03b10b45234adb Mon Sep 17 00:00:00 2001 From: Ivan Dlugos <6349682+vaind@users.noreply.github.com> Date: Thu, 10 Mar 2022 15:00:26 +0100 Subject: [PATCH 9/9] Apply suggestions from code review Co-authored-by: Bruno Garcia --- src/Sentry.Unity.Native/SentryNative.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Sentry.Unity.Native/SentryNative.cs b/src/Sentry.Unity.Native/SentryNative.cs index d635a04c8..5c5624a6b 100644 --- a/src/Sentry.Unity.Native/SentryNative.cs +++ b/src/Sentry.Unity.Native/SentryNative.cs @@ -9,7 +9,7 @@ namespace Sentry.Unity.Native /// public static class SentryNative { - private static Dictionary perDirectoryCrashInfo = new Dictionary(); + private static Dictionary _perDirectoryCrashInfo = new Dictionary(); /// /// Configures the native SDK. @@ -33,12 +33,15 @@ public static void Configure(SentryUnityOptions options) // on subsequent runs. Therefore, we cache the value during the whole runtime of the application. var cacheDirectory = SentryNativeBridge.GetCacheDirectory(options); bool crashedLastRun = false; - lock (perDirectoryCrashInfo) + // In the event the SDK is re-initialized with a different path on disk, we need to track which ones were already read + // Similarly we need to cache the value of each call since a subsequent call would return a different value + // as the file used on disk to mark it as crashed is deleted after we read it. + lock (_perDirectoryCrashInfo) { - if (!perDirectoryCrashInfo.TryGetValue(cacheDirectory, out crashedLastRun)) + if (!_perDirectoryCrashInfo.TryGetValue(cacheDirectory, out crashedLastRun)) { crashedLastRun = SentryNativeBridge.HandleCrashedLastRun(options); - perDirectoryCrashInfo.Add(cacheDirectory, crashedLastRun); + _perDirectoryCrashInfo.Add(cacheDirectory, crashedLastRun); options.DiagnosticLogger? .LogDebug("Native SDK reported: 'crashedLastRun': '{0}'", crashedLastRun);