From f83dc560af9c1e65d131220a93bf1acfd00eee30 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Thu, 10 Apr 2025 16:42:00 +0200 Subject: [PATCH 01/18] test timeout to see what CI does --- src/Sentry.Unity.Android/SentryNativeAndroid.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Sentry.Unity.Android/SentryNativeAndroid.cs b/src/Sentry.Unity.Android/SentryNativeAndroid.cs index 1d2122939..995cec9d7 100644 --- a/src/Sentry.Unity.Android/SentryNativeAndroid.cs +++ b/src/Sentry.Unity.Android/SentryNativeAndroid.cs @@ -43,7 +43,7 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry options.DiagnosticLogger?.LogDebug("Checking whether the Android SDK has already been initialized"); - if (SentryJava.IsEnabled(JniExecutor, TimeSpan.FromMilliseconds(200))) + if (SentryJava.IsEnabled(JniExecutor, TimeSpan.FromSeconds(10))) { options.DiagnosticLogger?.LogDebug("The Android SDK is already initialized"); } @@ -52,11 +52,11 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry options.DiagnosticLogger?.LogInfo("Initializing the Android SDK"); // Local testing had Init at an average of about 25ms. - SentryJava.Init(JniExecutor, options, TimeSpan.FromMilliseconds(200)); + SentryJava.Init(JniExecutor, options, TimeSpan.FromSeconds(10)); options.DiagnosticLogger?.LogDebug("Validating Android SDK initialization"); - if (!SentryJava.IsEnabled(JniExecutor, TimeSpan.FromMilliseconds(200))) + if (!SentryJava.IsEnabled(JniExecutor, TimeSpan.FromSeconds(10))) { options.DiagnosticLogger?.LogError("Failed to initialize Android Native Support"); return; From 569590e04b98a617b656f84cfac110d4bc5f3564 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Fri, 11 Apr 2025 11:37:00 +0200 Subject: [PATCH 02/18] reworked jni exeutor --- .../AndroidJavaScopeObserver.cs | 99 ++------ .../AndroidOptionConfiguration.cs | 36 +++ src/Sentry.Unity.Android/JniExecutor.cs | 225 +++++++++--------- .../NativeContextWriter.cs | 5 +- src/Sentry.Unity.Android/ScopeCallback.cs | 38 +++ src/Sentry.Unity.Android/SentryJava.cs | 180 ++++++++------ .../SentryNativeAndroid.cs | 38 ++- .../SentryNativeAndroidTests.cs | 1 - .../TestSentryJava.cs | 24 +- 9 files changed, 355 insertions(+), 291 deletions(-) create mode 100644 src/Sentry.Unity.Android/AndroidOptionConfiguration.cs create mode 100644 src/Sentry.Unity.Android/ScopeCallback.cs diff --git a/src/Sentry.Unity.Android/AndroidJavaScopeObserver.cs b/src/Sentry.Unity.Android/AndroidJavaScopeObserver.cs index 199de6498..147a01193 100644 --- a/src/Sentry.Unity.Android/AndroidJavaScopeObserver.cs +++ b/src/Sentry.Unity.Android/AndroidJavaScopeObserver.cs @@ -9,96 +9,31 @@ namespace Sentry.Unity.Android; /// internal class AndroidJavaScopeObserver : ScopeObserver { - private readonly IJniExecutor _jniExecutor; + private ISentryJava _sentryJava; - public AndroidJavaScopeObserver(SentryOptions options, IJniExecutor jniExecutor) : base("Android", options) + public AndroidJavaScopeObserver(SentryOptions options, ISentryJava sentryJava) : base("Android", options) { - _jniExecutor = jniExecutor; + _sentryJava = sentryJava; } - private static AndroidJavaObject GetSentryJava() => new AndroidJavaClass("io.sentry.Sentry"); + public override void AddBreadcrumbImpl(Breadcrumb breadcrumb) => + _sentryJava.AddBreadCrumb(breadcrumb); - private static AndroidJavaObject GetInternalSentryJava() => new AndroidJavaClass("io.sentry.android.core.InternalSentrySdk"); + public override void SetExtraImpl(string key, string? value) => + _sentryJava.SetExtra(key, value); - public override void AddBreadcrumbImpl(Breadcrumb breadcrumb) - { - _jniExecutor.Run(() => - { - using var sentry = GetSentryJava(); - using var javaBreadcrumb = new AndroidJavaObject("io.sentry.Breadcrumb"); - javaBreadcrumb.Set("message", breadcrumb.Message); - javaBreadcrumb.Set("type", breadcrumb.Type); - javaBreadcrumb.Set("category", breadcrumb.Category); - using var javaLevel = breadcrumb.Level.ToJavaSentryLevel(); - javaBreadcrumb.Set("level", javaLevel); - sentry.CallStatic("addBreadcrumb", javaBreadcrumb, null); - }); - } + public override void SetTagImpl(string key, string value) => + _sentryJava.SetTag(key, value); - public override void SetExtraImpl(string key, string? value) - { - _jniExecutor.Run(() => - { - using var sentry = GetSentryJava(); - sentry.CallStatic("setExtra", key, value); - }); - } - public override void SetTagImpl(string key, string value) - { - _jniExecutor.Run(() => - { - using var sentry = GetSentryJava(); - sentry.CallStatic("setTag", key, value); - }); - } + public override void UnsetTagImpl(string key) => + _sentryJava.UnsetTag(key); - public override void UnsetTagImpl(string key) - { - _jniExecutor.Run(() => - { - using var sentry = GetSentryJava(); - sentry.CallStatic("removeTag", key); - }); - } + public override void SetUserImpl(SentryUser user) => + _sentryJava.SetUser(user); - public override void SetUserImpl(SentryUser user) - { - _jniExecutor.Run(() => - { - AndroidJavaObject? javaUser = null; - try - { - javaUser = new AndroidJavaObject("io.sentry.protocol.User"); - javaUser.Set("email", user.Email); - javaUser.Set("id", user.Id); - javaUser.Set("username", user.Username); - javaUser.Set("ipAddress", user.IpAddress); - using var sentry = GetSentryJava(); - sentry.CallStatic("setUser", javaUser); - } - finally - { - javaUser?.Dispose(); - } - }); - } + public override void UnsetUserImpl() => + _sentryJava.UnsetUser(); - public override void UnsetUserImpl() - { - _jniExecutor.Run(() => - { - using var sentry = GetSentryJava(); - sentry.CallStatic("setUser", null); - }); - } - - public override void SetTraceImpl(SentryId traceId, SpanId spanId) - { - _jniExecutor.Run(() => - { - using var sentry = GetInternalSentryJava(); - // We have to explicitly cast to `(Double?)` - sentry.CallStatic("setTrace", traceId.ToString(), spanId.ToString(), (Double?)null, (Double?)null); - }); - } + public override void SetTraceImpl(SentryId traceId, SpanId spanId) => + _sentryJava.SetTrace(traceId, spanId); } diff --git a/src/Sentry.Unity.Android/AndroidOptionConfiguration.cs b/src/Sentry.Unity.Android/AndroidOptionConfiguration.cs new file mode 100644 index 000000000..6ca3e8143 --- /dev/null +++ b/src/Sentry.Unity.Android/AndroidOptionConfiguration.cs @@ -0,0 +1,36 @@ +using System; +using Sentry.Extensibility; +using UnityEngine; + +namespace Sentry.Unity.Android; + +internal class AndroidOptionsConfiguration : AndroidJavaProxy +{ + private readonly Action _callback; + private readonly IDiagnosticLogger? _logger; + + public AndroidOptionsConfiguration(Action callback, IDiagnosticLogger? logger) + : base("io.sentry.Sentry$OptionsConfiguration") + { + _callback = callback; + _logger = logger; + } + + public override AndroidJavaObject? Invoke(string methodName, AndroidJavaObject[] args) + { + try + { + if (methodName != "configure" || args.Length != 1) + { + throw new Exception($"Invalid invocation: {methodName}({args.Length} args)"); + } + + _callback(args[0]); + } + catch (Exception e) + { + _logger?.LogError(e, "Error invoking {0} ’.", methodName); + } + return null; + } +} diff --git a/src/Sentry.Unity.Android/JniExecutor.cs b/src/Sentry.Unity.Android/JniExecutor.cs index 079484b6d..dbb36eba8 100644 --- a/src/Sentry.Unity.Android/JniExecutor.cs +++ b/src/Sentry.Unity.Android/JniExecutor.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Concurrent; using System.Threading; using System.Threading.Tasks; using Sentry.Extensibility; @@ -8,150 +9,165 @@ namespace Sentry.Unity.Android; internal class JniExecutor : IJniExecutor { - // We're capping out at 16ms - 1 frame at 60 frames per second - private static readonly TimeSpan DefaultTimeout = TimeSpan.FromMilliseconds(16); - - private readonly CancellationTokenSource _shutdownSource; - private readonly AutoResetEvent _taskEvent; + private static readonly TimeSpan DefaultTimeout = TimeSpan.FromSeconds(3); + + private readonly ConcurrentQueue _operationQueue = new ConcurrentQueue(); + private readonly AutoResetEvent _queueSignal = new AutoResetEvent(false); private readonly IDiagnosticLogger? _logger; + private readonly CancellationTokenSource _shutdownSource = new CancellationTokenSource(); - private Delegate _currentTask = null!; // The current task will always be set together with the task event - - private TaskCompletionSource? _taskCompletionSource; + private Thread? _workerThread; + private bool _isDisposed; - private readonly object _lock = new object(); + private class JniOperation + { + public Delegate Action { get; } + public TaskCompletionSource? CompletionSource { get; } - private bool _isDisposed; - private Thread? _workerThread; + public JniOperation(Delegate action, TaskCompletionSource? completionSource = null) + { + Action = action; + CompletionSource = completionSource; + } + } public JniExecutor(IDiagnosticLogger? logger) { _logger = logger; - _taskEvent = new AutoResetEvent(false); - _shutdownSource = new CancellationTokenSource(); - - _workerThread = new Thread(DoWork) { IsBackground = true, Name = "SentryJniExecutorThread" }; + _workerThread = new Thread(ProcessJniQueue) { IsBackground = true, Name = "SentryJniExecutorThread" }; _workerThread.Start(); } - private void DoWork() + private void ProcessJniQueue() { AndroidJNI.AttachCurrentThread(); - var waitHandles = new[] { _taskEvent, _shutdownSource.Token.WaitHandle }; + var waitHandles = new[] { _queueSignal, _shutdownSource.Token.WaitHandle }; while (!_isDisposed) { var index = WaitHandle.WaitAny(waitHandles); - if (index > 0) + if (index > 0 || _isDisposed) { - // We only care about the _taskEvent break; } - try + while (!_isDisposed && _operationQueue.TryDequeue(out var operation)) { - // Matching the type of the `_currentTask` exactly. The result gets cast to the expected type - // when returning from the blocking call. - switch (_currentTask) + try { - case Action action: - { - action.Invoke(); - _taskCompletionSource?.SetResult(null); - break; - } - case Func func1: - { - var result = func1.Invoke(); - _taskCompletionSource?.SetResult(result); - break; - } - case Func func2: - { - var result = func2.Invoke(); - _taskCompletionSource?.SetResult(result); - break; - } - case Func func3: - { - var result = func3.Invoke(); - _taskCompletionSource?.SetResult(result); - break; - } - default: - throw new NotImplementedException($"Task type '{_currentTask?.GetType()}' with value '{_currentTask}' is not implemented in the JniExecutor."); + ExecuteJniOperation(operation); + } + catch (Exception ex) + { + _logger?.LogError(ex, "Error executing JNI operation"); + operation.CompletionSource?.TrySetException(ex); } - } - catch (Exception e) - { - _logger?.LogError(e, "Error during JNI execution."); - _taskCompletionSource?.SetException(e); } } AndroidJNI.DetachCurrentThread(); + _logger?.LogDebug("JNI executor thread terminated"); } - public TResult? Run(Func jniOperation, TimeSpan? timeout = null) + private void ExecuteJniOperation(JniOperation operation) { - lock (_lock) + try { - timeout ??= DefaultTimeout; - using var timeoutCts = new CancellationTokenSource(timeout.Value); - _taskCompletionSource = new TaskCompletionSource(); - _currentTask = jniOperation; - _taskEvent.Set(); - - try - { - _taskCompletionSource.Task.Wait(timeoutCts.Token); - return (TResult?)_taskCompletionSource.Task.Result; - } - catch (OperationCanceledException) + switch (operation.Action) { - _logger?.LogError("JNI execution timed out."); - return default; + case Action action: + action.Invoke(); + operation.CompletionSource?.TrySetResult(null); + break; + case Func func1: + var result1 = func1.Invoke(); + operation.CompletionSource?.TrySetResult(result1); + break; + case Func func2: + var result2 = func2.Invoke(); + operation.CompletionSource?.TrySetResult(result2); + break; + case Func func3: + var result3 = func3.Invoke(); + operation.CompletionSource?.TrySetResult(result3); + break; + default: + var error = new NotImplementedException($"Task type '{operation.Action.GetType()}' is not implemented in JniExecutor"); + _logger?.LogError(error, "Unsupported JNI operation type"); + operation.CompletionSource?.TrySetException(error); + break; } - catch (Exception e) - { - _logger?.LogError(e, "Error during JNI execution."); - return default; - } - finally + } + catch (Exception ex) + { + _logger?.LogError(ex, "Error during JNI operation execution"); + operation.CompletionSource?.TrySetException(ex); + } + } + + /// + /// Runs a JNI operation that returns a result, waiting for completion with a timeout + /// + public TResult? Run(Func jniOperation, TimeSpan? timeout = null) + { + timeout ??= DefaultTimeout; + var completionSource = new TaskCompletionSource(); + var operation = new JniOperation(jniOperation, completionSource); + + _operationQueue.Enqueue(operation); + _queueSignal.Set(); + + try + { + if (completionSource.Task.Wait(timeout.Value)) { - _currentTask = null!; + return (TResult?)completionSource.Task.Result; } + + _logger?.LogError("JNI operation timed out after {0}ms", timeout.Value.TotalMilliseconds); + return default; + } + catch (Exception ex) + { + _logger?.LogError(ex, "Error waiting for JNI operation result"); + return default; } } + /// + /// Runs a JNI operation with no return value, waiting for completion with a timeout + /// public void Run(Action jniOperation, TimeSpan? timeout = null) { - lock (_lock) - { - timeout ??= DefaultTimeout; - using var timeoutCts = new CancellationTokenSource(timeout.Value); - _taskCompletionSource = new TaskCompletionSource(); - _currentTask = jniOperation; - _taskEvent.Set(); + timeout ??= DefaultTimeout; + var completionSource = new TaskCompletionSource(); + var operation = new JniOperation(jniOperation, completionSource); + + _operationQueue.Enqueue(operation); + _queueSignal.Set(); - try - { - _taskCompletionSource.Task.Wait(timeoutCts.Token); - } - catch (OperationCanceledException) - { - _logger?.LogError("JNI execution timed out."); - } - catch (Exception e) - { - _logger?.LogError(e, "Error during JNI execution."); - } - finally + try + { + if (!completionSource.Task.Wait(timeout.Value)) { - _currentTask = null!; + _logger?.LogError("JNI operation timed out after {0}ms", timeout.Value.TotalMilliseconds); } } + catch (Exception ex) + { + _logger?.LogError(ex, "Error waiting for JNI operation completion"); + } + } + + /// + /// Runs a JNI operation without waiting for completion - true fire and forget + /// + public void RunAsync(Action jniOperation) + { + var operation = new JniOperation(jniOperation); + _operationQueue.Enqueue(operation); + _queueSignal.Set(); } public void Dispose() @@ -162,21 +178,18 @@ public void Dispose() } _isDisposed = true; - _shutdownSource.Cancel(); + try { _workerThread?.Join(100); } - catch (ThreadStateException) - { - _logger?.LogError("JNI Executor Worker thread was never started during disposal"); - } - catch (ThreadInterruptedException) + catch (Exception ex) { - _logger?.LogError("JNI Executor Worker thread was interrupted during disposal"); + _logger?.LogError(ex, "Error during JNI executor thread shutdown"); } - _taskEvent.Dispose(); + _queueSignal.Dispose(); + _shutdownSource.Dispose(); } } diff --git a/src/Sentry.Unity.Android/NativeContextWriter.cs b/src/Sentry.Unity.Android/NativeContextWriter.cs index d1db73e4e..573a8f2e6 100644 --- a/src/Sentry.Unity.Android/NativeContextWriter.cs +++ b/src/Sentry.Unity.Android/NativeContextWriter.cs @@ -4,12 +4,10 @@ namespace Sentry.Unity.Android; internal class NativeContextWriter : ContextWriter { - private readonly IJniExecutor _jniExecutor; private readonly ISentryJava _sentryJava; - public NativeContextWriter(IJniExecutor jniExecutor, ISentryJava sentryJava) + public NativeContextWriter(ISentryJava sentryJava) { - _jniExecutor = jniExecutor; _sentryJava = sentryJava; } @@ -53,7 +51,6 @@ protected override void WriteScope( // the "unity" context, but it doesn't seem so useful and the effort to do is larger because there's no // class for it in Java - not sure how we could add a generic context object in Java... _sentryJava.WriteScope( - _jniExecutor, GpuId, GpuName, GpuVendorName, diff --git a/src/Sentry.Unity.Android/ScopeCallback.cs b/src/Sentry.Unity.Android/ScopeCallback.cs new file mode 100644 index 000000000..78c2dae99 --- /dev/null +++ b/src/Sentry.Unity.Android/ScopeCallback.cs @@ -0,0 +1,38 @@ +// Implements the io.sentry.ScopeCallback interface. + +using System; +using UnityEngine; + +namespace Sentry.Unity.Android; + +internal class ScopeCallback : AndroidJavaProxy +{ + private readonly Action _callback; + + public ScopeCallback(Action callback) : base("io.sentry.ScopeCallback") + { + _callback = callback; + } + + // Note: defining the method should be enough with the default Invoke(), but in reality it doesn't work: + // No such proxy method: Sentry.Unity.Android.SentryJava+ScopeCallback.run(UnityEngine.AndroidJavaObject) + // public void run(AndroidJavaObject scope) => UnityEngine.Debug.Log("run() invoked"); + // Therefore, we're overriding the Invoke() instead: + public override AndroidJavaObject? Invoke(string methodName, AndroidJavaObject[] args) + { + try + { + if (methodName != "run" || args.Length != 1) + { + throw new Exception($"Invalid invocation: {methodName}({args.Length} args)"); + } + _callback(args[0]); + } + catch (Exception e) + { + // Adding the Sentry logger tag ensures we don't send this error to Sentry. + Debug.unityLogger.Log(LogType.Error, UnityLogger.LogTag, $"Error in SentryJava.ScopeCallback: {e}"); + } + return null; + } +} diff --git a/src/Sentry.Unity.Android/SentryJava.cs b/src/Sentry.Unity.Android/SentryJava.cs index 679be6d71..c386c1de5 100644 --- a/src/Sentry.Unity.Android/SentryJava.cs +++ b/src/Sentry.Unity.Android/SentryJava.cs @@ -8,13 +8,12 @@ namespace Sentry.Unity.Android; internal interface ISentryJava { - public bool IsEnabled(IJniExecutor jniExecutor, TimeSpan timeout); - public void Init(IJniExecutor jniExecutor, SentryUnityOptions options, TimeSpan timeout); - public string? GetInstallationId(IJniExecutor jniExecutor); - public bool? CrashedLastRun(IJniExecutor jniExecutor); - public void Close(IJniExecutor jniExecutor); + public bool IsEnabled(TimeSpan timeout); + public void Init(SentryUnityOptions options, TimeSpan timeout); + public string? GetInstallationId(); + public bool? CrashedLastRun(); + public void Close(); public void WriteScope( - IJniExecutor jniExecutor, int? GpuId, string? GpuName, string? GpuVendorName, @@ -31,6 +30,15 @@ public void WriteScope( bool? GpuMultiThreadedRendering, string? GpuGraphicsShaderLevel); public bool IsSentryJavaPresent(); + + // Methods for the ScopeObserver + public void AddBreadCrumb(Breadcrumb breadcrumb); + public void SetExtra(string key, string? value); + public void SetTag(string key, string? value); + public void UnsetTag(string key); + public void SetUser(SentryUser user); + public void UnsetUser(); + public void SetTrace(SentryId traceId, SpanId spanId); } /// @@ -43,20 +51,32 @@ public void WriteScope( /// internal class SentryJava : ISentryJava { + private readonly JniExecutor? _jniExecutor; private static AndroidJavaObject GetSentryJava() => new AndroidJavaClass("io.sentry.Sentry"); + private static AndroidJavaObject GetInternalSentryJava() => new AndroidJavaClass("io.sentry.android.core.InternalSentrySdk"); + + public SentryJava(IDiagnosticLogger? logger) + { + _jniExecutor = new JniExecutor(logger); + } - public bool IsEnabled(IJniExecutor jniExecutor, TimeSpan timeout) + public bool IsEnabled(TimeSpan timeout) { - return jniExecutor.Run(() => + if (_jniExecutor is null) + { + return false; + } + + return _jniExecutor.Run(() => { using var sentry = GetSentryJava(); return sentry.CallStatic("isEnabled"); }, timeout); } - public void Init(IJniExecutor jniExecutor, SentryUnityOptions options, TimeSpan timeout) + public void Init(SentryUnityOptions options, TimeSpan timeout) { - jniExecutor.Run(() => + _jniExecutor?.Run(() => { using var sentry = new AndroidJavaClass("io.sentry.android.core.SentryAndroid"); using var context = new AndroidJavaClass("com.unity3d.player.UnityPlayer") @@ -99,40 +119,9 @@ public void Init(IJniExecutor jniExecutor, SentryUnityOptions options, TimeSpan }, timeout); } - internal class AndroidOptionsConfiguration : AndroidJavaProxy + public string? GetInstallationId() { - private readonly Action _callback; - private readonly IDiagnosticLogger? _logger; - - public AndroidOptionsConfiguration(Action callback, IDiagnosticLogger? logger) - : base("io.sentry.Sentry$OptionsConfiguration") - { - _callback = callback; - _logger = logger; - } - - public override AndroidJavaObject? Invoke(string methodName, AndroidJavaObject[] args) - { - try - { - if (methodName != "configure" || args.Length != 1) - { - throw new Exception($"Invalid invocation: {methodName}({args.Length} args)"); - } - - _callback(args[0]); - } - catch (Exception e) - { - _logger?.LogError(e, "Error invoking {0} ’.", methodName); - } - return null; - } - } - - public string? GetInstallationId(IJniExecutor jniExecutor) - { - return jniExecutor.Run(() => + return _jniExecutor?.Run(() => { using var sentry = GetSentryJava(); using var hub = sentry.CallStatic("getCurrentHub"); @@ -142,7 +131,7 @@ public AndroidOptionsConfiguration(Action callback, IDiagnost } /// - /// Returns whether or not the last run resulted in a crash. + /// Returns whether the last run resulted in a crash. /// /// /// This value is returned by the Android SDK and reports for both ART and NDK. @@ -151,9 +140,9 @@ public AndroidOptionsConfiguration(Action callback, IDiagnost /// True if the last run terminated in a crash. No otherwise. /// If the SDK wasn't able to find this information, null is returned. /// - public bool? CrashedLastRun(IJniExecutor jniExecutor) + public bool? CrashedLastRun() { - return jniExecutor.Run(() => + return _jniExecutor?.Run(() => { using var sentry = GetSentryJava(); using var jo = sentry.CallStatic("isCrashedLastRun"); @@ -161,9 +150,9 @@ public AndroidOptionsConfiguration(Action callback, IDiagnost }); } - public void Close(IJniExecutor jniExecutor) + public void Close() { - jniExecutor.Run(() => + _jniExecutor?.Run(() => { using var sentry = GetSentryJava(); sentry.CallStatic("close"); @@ -171,7 +160,6 @@ public void Close(IJniExecutor jniExecutor) } public void WriteScope( - IJniExecutor jniExecutor, int? GpuId, string? GpuName, string? GpuVendorName, @@ -188,7 +176,7 @@ public void WriteScope( bool? GpuMultiThreadedRendering, string? GpuGraphicsShaderLevel) { - jniExecutor.Run(() => + _jniExecutor?.Run(() => { using var gpu = new AndroidJavaObject("io.sentry.protocol.Gpu"); gpu.SetIfNotNull("name", GpuName); @@ -223,37 +211,87 @@ public bool IsSentryJavaPresent() return true; } - // Implements the io.sentry.ScopeCallback interface. - internal class ScopeCallback : AndroidJavaProxy + public void AddBreadCrumb(Breadcrumb breadcrumb) { - private readonly Action _callback; + _jniExecutor?.RunAsync(() => + { + using var sentry = GetSentryJava(); + using var javaBreadcrumb = new AndroidJavaObject("io.sentry.Breadcrumb"); + javaBreadcrumb.Set("message", breadcrumb.Message); + javaBreadcrumb.Set("type", breadcrumb.Type); + javaBreadcrumb.Set("category", breadcrumb.Category); + using var javaLevel = breadcrumb.Level.ToJavaSentryLevel(); + javaBreadcrumb.Set("level", javaLevel); + sentry.CallStatic("addBreadcrumb", javaBreadcrumb, null); + }); + } - public ScopeCallback(Action callback) : base("io.sentry.ScopeCallback") + public void SetExtra(string key, string? value) + { + _jniExecutor?.RunAsync(() => { - _callback = callback; - } + using var sentry = GetSentryJava(); + sentry.CallStatic("setExtra", key, value); + }); + } + + public void SetTag(string key, string? value) + { + _jniExecutor?.RunAsync(() => + { + using var sentry = GetSentryJava(); + sentry.CallStatic("setTag", key, value); + }); + } + + public void UnsetTag(string key) + { + _jniExecutor?.RunAsync(() => + { + using var sentry = GetSentryJava(); + sentry.CallStatic("removeTag", key); + }); + } - // Note: defining the method should be enough with the default Invoke(), but in reality it doesn't work: - // No such proxy method: Sentry.Unity.Android.SentryJava+ScopeCallback.run(UnityEngine.AndroidJavaObject) - // public void run(AndroidJavaObject scope) => UnityEngine.Debug.Log("run() invoked"); - // Therefore, we're overriding the Invoke() instead: - public override AndroidJavaObject? Invoke(string methodName, AndroidJavaObject[] args) + public void SetUser(SentryUser user) + { + _jniExecutor?.RunAsync(() => { + AndroidJavaObject? javaUser = null; try { - if (methodName != "run" || args.Length != 1) - { - throw new Exception($"Invalid invocation: {methodName}({args.Length} args)"); - } - _callback(args[0]); + javaUser = new AndroidJavaObject("io.sentry.protocol.User"); + javaUser.Set("email", user.Email); + javaUser.Set("id", user.Id); + javaUser.Set("username", user.Username); + javaUser.Set("ipAddress", user.IpAddress); + using var sentry = GetSentryJava(); + sentry.CallStatic("setUser", javaUser); } - catch (Exception e) + finally { - // Adding the Sentry logger tag ensures we don't send this error to Sentry. - Debug.unityLogger.Log(LogType.Error, UnityLogger.LogTag, $"Error in SentryJava.ScopeCallback: {e}"); + javaUser?.Dispose(); } - return null; - } + }); + } + + public void UnsetUser() + { + _jniExecutor?.RunAsync(() => + { + using var sentry = GetSentryJava(); + sentry.CallStatic("setUser", null); + }); + } + + public void SetTrace(SentryId traceId, SpanId spanId) + { + _jniExecutor?.RunAsync(() => + { + using var sentry = GetInternalSentryJava(); + // We have to explicitly cast to `(Double?)` + sentry.CallStatic("setTrace", traceId.ToString(), spanId.ToString(), (Double?)null, (Double?)null); + }); } // https://github.com/getsentry/sentry-java/blob/db4dfc92f202b1cefc48d019fdabe24d487db923/sentry/src/main/java/io/sentry/SentryLevel.java#L4-L9 diff --git a/src/Sentry.Unity.Android/SentryNativeAndroid.cs b/src/Sentry.Unity.Android/SentryNativeAndroid.cs index 995cec9d7..f1017e18a 100644 --- a/src/Sentry.Unity.Android/SentryNativeAndroid.cs +++ b/src/Sentry.Unity.Android/SentryNativeAndroid.cs @@ -11,8 +11,7 @@ namespace Sentry.Unity.Android; /// public static class SentryNativeAndroid { - internal static IJniExecutor? JniExecutor; - internal static ISentryJava SentryJava = new SentryJava(); + internal static ISentryJava? SentryJava; /// /// Configures the native Android support. @@ -30,6 +29,7 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry options.DiagnosticLogger?.LogDebug("Checking whether the Android SDK is present."); + SentryJava = new SentryJava(options.DiagnosticLogger); if (!SentryJava.IsSentryJavaPresent()) { options.DiagnosticLogger?.LogError("Android Native Support has been enabled but the " + @@ -39,11 +39,9 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry return; } - JniExecutor ??= new JniExecutor(options.DiagnosticLogger); - options.DiagnosticLogger?.LogDebug("Checking whether the Android SDK has already been initialized"); - if (SentryJava.IsEnabled(JniExecutor, TimeSpan.FromSeconds(10))) + if (SentryJava.IsEnabled( TimeSpan.FromMilliseconds(200))) { options.DiagnosticLogger?.LogDebug("The Android SDK is already initialized"); } @@ -52,11 +50,11 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry options.DiagnosticLogger?.LogInfo("Initializing the Android SDK"); // Local testing had Init at an average of about 25ms. - SentryJava.Init(JniExecutor, options, TimeSpan.FromSeconds(10)); + SentryJava.Init(options, TimeSpan.FromMilliseconds(200)); options.DiagnosticLogger?.LogDebug("Validating Android SDK initialization"); - if (!SentryJava.IsEnabled(JniExecutor, TimeSpan.FromSeconds(10))) + if (!SentryJava.IsEnabled(TimeSpan.FromMilliseconds(200))) { options.DiagnosticLogger?.LogError("Failed to initialize Android Native Support"); return; @@ -65,14 +63,14 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry options.DiagnosticLogger?.LogDebug("Configuring scope sync"); - options.NativeContextWriter = new NativeContextWriter(JniExecutor, SentryJava); - options.ScopeObserver = new AndroidJavaScopeObserver(options, JniExecutor); + options.NativeContextWriter = new NativeContextWriter(SentryJava); + options.ScopeObserver = new AndroidJavaScopeObserver(options, SentryJava); options.EnableScopeSync = true; options.CrashedLastRun = () => { options.DiagnosticLogger?.LogDebug("Checking for 'CrashedLastRun'"); - var crashedLastRun = SentryJava.CrashedLastRun(JniExecutor); + var crashedLastRun = SentryJava.CrashedLastRun(); if (crashedLastRun is null) { // Could happen if the Android SDK wasn't initialized before the .NET layer. @@ -107,7 +105,7 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry options.DiagnosticLogger?.LogDebug("Fetching installation ID"); - options.DefaultUserId = SentryJava.GetInstallationId(JniExecutor); + options.DefaultUserId = SentryJava.GetInstallationId(); if (string.IsNullOrEmpty(options.DefaultUserId)) { // In case we can't get an installation ID we create one and sync that down to the native layer @@ -141,19 +139,19 @@ internal static void Close(SentryUnityOptions options, ISentryUnityInfo sentryUn { options.DiagnosticLogger?.LogInfo("Attempting to close the Android SDK"); - if (!sentryUnityInfo.IsNativeSupportEnabled(options, platform) || !SentryJava.IsSentryJavaPresent()) + if (!sentryUnityInfo.IsNativeSupportEnabled(options, platform)) { - options.DiagnosticLogger?.LogDebug("Android Native Support is not enable. Skipping closing the Android SDK"); + options.DiagnosticLogger?.LogDebug("Android Native Support is not enabled. Skipping closing the Android SDK"); return; } - // Sentry Native is initialized and closed by the Java SDK, no need to call into it directly - options.DiagnosticLogger?.LogDebug("Closing the Android SDK"); + if (SentryJava?.IsSentryJavaPresent() is not true) + { + options.DiagnosticLogger?.LogDebug("Failed to find Sentry Java. Skipping closing the Android SDK"); + return; + } - // This is an edge-case where the Android SDK has been enabled and setup during build-time but is being - // shut down at runtime. In this case Configure() has not been called and there is no JniExecutor yet - JniExecutor ??= new JniExecutor(options.DiagnosticLogger); - SentryJava?.Close(JniExecutor); - JniExecutor.Dispose(); + options.DiagnosticLogger?.LogDebug("Closing the Android SDK"); + SentryJava.Close(); } } diff --git a/test/Sentry.Unity.Android.Tests/SentryNativeAndroidTests.cs b/test/Sentry.Unity.Android.Tests/SentryNativeAndroidTests.cs index f75c6f0b1..d52e90650 100644 --- a/test/Sentry.Unity.Android.Tests/SentryNativeAndroidTests.cs +++ b/test/Sentry.Unity.Android.Tests/SentryNativeAndroidTests.cs @@ -28,7 +28,6 @@ public void SetUp() _reinstallCalled = false; _sentryUnityInfo = new TestUnityInfo { IL2CPP = false }; - SentryNativeAndroid.JniExecutor ??= new JniExecutor(_logger); _testSentryJava = new TestSentryJava(); SentryNativeAndroid.SentryJava = _testSentryJava; diff --git a/test/Sentry.Unity.Android.Tests/TestSentryJava.cs b/test/Sentry.Unity.Android.Tests/TestSentryJava.cs index be1139782..113079033 100644 --- a/test/Sentry.Unity.Android.Tests/TestSentryJava.cs +++ b/test/Sentry.Unity.Android.Tests/TestSentryJava.cs @@ -10,19 +10,17 @@ internal class TestSentryJava : ISentryJava public string? InstallationId { get; set; } public bool? IsCrashedLastRun { get; set; } - public bool IsEnabled(IJniExecutor jniExecutor, TimeSpan timeout) => Enabled; + public bool IsEnabled(TimeSpan timeout) => Enabled; - public void Init(IJniExecutor jniExecutor, SentryUnityOptions options, TimeSpan timeout) { } + public void Init(SentryUnityOptions options, TimeSpan timeout) { } - public string? GetInstallationId(IJniExecutor jniExecutor) => InstallationId; + public string? GetInstallationId() => InstallationId; - public bool? CrashedLastRun(IJniExecutor jniExecutor) => IsCrashedLastRun; + public bool? CrashedLastRun() => IsCrashedLastRun; - public void Close(IJniExecutor jniExecutor) { } - public void SetTrace(IJniExecutor jniExecutor, string traceId, string spanId) { } + public void Close() { } public void WriteScope( - IJniExecutor jniExecutor, int? GpuId, string? GpuName, string? GpuVendorName, @@ -41,4 +39,16 @@ public void WriteScope( { } public bool IsSentryJavaPresent() => SentryPresent; + public void AddBreadCrumb(Breadcrumb breadcrumb) { } + + public void SetExtra(string key, string? value) { } + public void SetTag(string key, string? value) { } + + public void UnsetTag(string key) { } + + public void SetUser(SentryUser user) { } + + public void UnsetUser() { } + + public void SetTrace(SentryId traceId, SpanId spanId) { } } From afa8af70293ad51019fdc8f08d0268d416447980 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Fri, 11 Apr 2025 12:02:29 +0200 Subject: [PATCH 03/18] fixed bug and tests --- src/Sentry.Unity.Android/JniExecutor.cs | 22 +++----- .../SentryNativeAndroid.cs | 5 +- .../JniExecutorTests.cs | 53 ++++++++++++++++++- 3 files changed, 62 insertions(+), 18 deletions(-) diff --git a/src/Sentry.Unity.Android/JniExecutor.cs b/src/Sentry.Unity.Android/JniExecutor.cs index dbb36eba8..d7654d5ae 100644 --- a/src/Sentry.Unity.Android/JniExecutor.cs +++ b/src/Sentry.Unity.Android/JniExecutor.cs @@ -10,7 +10,7 @@ namespace Sentry.Unity.Android; internal class JniExecutor : IJniExecutor { private static readonly TimeSpan DefaultTimeout = TimeSpan.FromSeconds(3); - + private readonly ConcurrentQueue _operationQueue = new ConcurrentQueue(); private readonly AutoResetEvent _queueSignal = new AutoResetEvent(false); private readonly IDiagnosticLogger? _logger; @@ -54,15 +54,7 @@ private void ProcessJniQueue() while (!_isDisposed && _operationQueue.TryDequeue(out var operation)) { - try - { - ExecuteJniOperation(operation); - } - catch (Exception ex) - { - _logger?.LogError(ex, "Error executing JNI operation"); - operation.CompletionSource?.TrySetException(ex); - } + ExecuteJniOperation(operation); } } @@ -101,7 +93,7 @@ private void ExecuteJniOperation(JniOperation operation) } catch (Exception ex) { - _logger?.LogError(ex, "Error during JNI operation execution"); + _logger?.LogError(ex, "Error during JNI operation execution."); operation.CompletionSource?.TrySetException(ex); } } @@ -114,7 +106,7 @@ private void ExecuteJniOperation(JniOperation operation) timeout ??= DefaultTimeout; var completionSource = new TaskCompletionSource(); var operation = new JniOperation(jniOperation, completionSource); - + _operationQueue.Enqueue(operation); _queueSignal.Set(); @@ -124,7 +116,7 @@ private void ExecuteJniOperation(JniOperation operation) { return (TResult?)completionSource.Task.Result; } - + _logger?.LogError("JNI operation timed out after {0}ms", timeout.Value.TotalMilliseconds); return default; } @@ -143,7 +135,7 @@ public void Run(Action jniOperation, TimeSpan? timeout = null) timeout ??= DefaultTimeout; var completionSource = new TaskCompletionSource(); var operation = new JniOperation(jniOperation, completionSource); - + _operationQueue.Enqueue(operation); _queueSignal.Set(); @@ -179,7 +171,7 @@ public void Dispose() _isDisposed = true; _shutdownSource.Cancel(); - + try { _workerThread?.Join(100); diff --git a/src/Sentry.Unity.Android/SentryNativeAndroid.cs b/src/Sentry.Unity.Android/SentryNativeAndroid.cs index f1017e18a..7b9b8e685 100644 --- a/src/Sentry.Unity.Android/SentryNativeAndroid.cs +++ b/src/Sentry.Unity.Android/SentryNativeAndroid.cs @@ -11,6 +11,8 @@ namespace Sentry.Unity.Android; /// public static class SentryNativeAndroid { + // This is an internal static field that gets overwritten during testing. We cannot have it as optional + // parameter on `Configure` due SentryNativeAndroid being public internal static ISentryJava? SentryJava; /// @@ -29,7 +31,8 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry options.DiagnosticLogger?.LogDebug("Checking whether the Android SDK is present."); - SentryJava = new SentryJava(options.DiagnosticLogger); + // Only overwriting if it has not been set yet (by tests) + SentryJava ??= new SentryJava(options.DiagnosticLogger); if (!SentryJava.IsSentryJavaPresent()) { options.DiagnosticLogger?.LogError("Android Native Support has been enabled but the " + diff --git a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs index 30e296e91..cc38ebbfd 100644 --- a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs +++ b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs @@ -1,6 +1,7 @@ using System; using System.Linq; using System.Threading; +using System.Threading.Tasks; using NUnit.Framework; using Sentry.Unity.Tests.SharedClasses; @@ -79,7 +80,7 @@ public void Run_WithTimeout_LogsErrorOnTimeout() // Assert Assert.IsTrue(_logger.Logs.Any(log => log.logLevel == SentryLevel.Error && - log.message.Contains("JNI execution timed out."))); + log.message.Contains("JNI operation timed out after "))); } [Test] @@ -95,7 +96,7 @@ public void Run_ThrowingOperation_LogsError() // Assert Assert.IsTrue(_logger.Logs.Any(log => log.logLevel == SentryLevel.Error && - log.message.Contains("Error during JNI execution."))); + log.message.Contains("Error during JNI operation execution."))); } [Test] @@ -110,5 +111,53 @@ public void Run_Generic_ReturnsDefaultOnException() // Assert Assert.That(result, Is.Null); } + + [Test] + public void RunAsync_Action_DoesNotWaitForCompletion() + { + // Arrange + var executed = false; + var completionSource = new TaskCompletionSource(); + var action = () => + { + Thread.Sleep(100); // Simulate long-running operation + executed = true; + completionSource.SetResult(true); + }; + + // Act + var stopwatch = new System.Diagnostics.Stopwatch(); + stopwatch.Start(); + _sut.RunAsync(action); + stopwatch.Stop(); + + // Assert + Assert.That(stopwatch.ElapsedMilliseconds, Is.LessThan(50), "RunAsync should return immediately"); + Assert.That(executed, Is.False, "Operation should not have completed yet"); + + // Wait for async operation to complete + Assert.That(completionSource.Task.Wait(500), Is.True, "Operation should eventually complete"); + Assert.That(executed, Is.True, "Operation should have completed after waiting"); + } + + [Test] + public void RunAsync_MultipleOperations_AllExecuteEventually() + { + // Arrange + var counter = 0; + var manualReset = new ManualResetEventSlim(); + + // Act + for (int i = 0; i < 5; i++) + { + _sut.RunAsync(() => Interlocked.Increment(ref counter)); + } + + // Wait a bit to allow operations to execute + Thread.Sleep(200); + + // Assert + Assert.That(counter, Is.EqualTo(5), "All operations should be executed"); + } } } From 147065ff15338a86a9678c5946a3a544051b55d9 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Fri, 11 Apr 2025 12:11:12 +0200 Subject: [PATCH 04/18] improved tests --- .../JniExecutorTests.cs | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs index cc38ebbfd..16aec96c7 100644 --- a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs +++ b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs @@ -118,25 +118,27 @@ public void RunAsync_Action_DoesNotWaitForCompletion() // Arrange var executed = false; var completionSource = new TaskCompletionSource(); - var action = () => + var fakeLongOperation = TimeSpan.FromMilliseconds(100); + + void Action() { - Thread.Sleep(100); // Simulate long-running operation + Thread.Sleep(fakeLongOperation); // Simulate long-running operation executed = true; completionSource.SetResult(true); - }; + } // Act var stopwatch = new System.Diagnostics.Stopwatch(); stopwatch.Start(); - _sut.RunAsync(action); + _sut.RunAsync(Action); stopwatch.Stop(); // Assert - Assert.That(stopwatch.ElapsedMilliseconds, Is.LessThan(50), "RunAsync should return immediately"); + Assert.That(stopwatch.Elapsed, Is.LessThan(fakeLongOperation), "RunAsync should return immediately"); Assert.That(executed, Is.False, "Operation should not have completed yet"); - // Wait for async operation to complete - Assert.That(completionSource.Task.Wait(500), Is.True, "Operation should eventually complete"); + // Wait for fake long work to complete + Assert.That(completionSource.Task.Wait(500), Is.True, "Operation should complete eventually"); Assert.That(executed, Is.True, "Operation should have completed after waiting"); } @@ -145,19 +147,24 @@ public void RunAsync_MultipleOperations_AllExecuteEventually() { // Arrange var counter = 0; - var manualReset = new ManualResetEventSlim(); + const int expectedCount = 5; + var countdownEvent = new CountdownEvent(expectedCount); // Act - for (int i = 0; i < 5; i++) + for (var i = 0; i < expectedCount; i++) { - _sut.RunAsync(() => Interlocked.Increment(ref counter)); + _sut.RunAsync(() => { + Interlocked.Increment(ref counter); + countdownEvent.Signal(); + }); } - // Wait a bit to allow operations to execute - Thread.Sleep(200); + // Wait with timeout instead of arbitrary sleep + var allOperationsCompleted = countdownEvent.Wait(TimeSpan.FromMilliseconds(500)); // Assert - Assert.That(counter, Is.EqualTo(5), "All operations should be executed"); + Assert.That(allOperationsCompleted, Is.True, "All operations should complete within the timeout"); + Assert.That(counter, Is.EqualTo(expectedCount)); } } } From 633f839a7011d588be592c7ca6fdefc8dc5c389a Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Fri, 11 Apr 2025 10:14:07 +0000 Subject: [PATCH 05/18] Format code --- src/Sentry.Unity.Android/SentryNativeAndroid.cs | 2 +- test/Sentry.Unity.Android.Tests/JniExecutorTests.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Sentry.Unity.Android/SentryNativeAndroid.cs b/src/Sentry.Unity.Android/SentryNativeAndroid.cs index 7b9b8e685..337260ec4 100644 --- a/src/Sentry.Unity.Android/SentryNativeAndroid.cs +++ b/src/Sentry.Unity.Android/SentryNativeAndroid.cs @@ -44,7 +44,7 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry options.DiagnosticLogger?.LogDebug("Checking whether the Android SDK has already been initialized"); - if (SentryJava.IsEnabled( TimeSpan.FromMilliseconds(200))) + if (SentryJava.IsEnabled(TimeSpan.FromMilliseconds(200))) { options.DiagnosticLogger?.LogDebug("The Android SDK is already initialized"); } diff --git a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs index 16aec96c7..d88d38fe2 100644 --- a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs +++ b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs @@ -153,7 +153,8 @@ public void RunAsync_MultipleOperations_AllExecuteEventually() // Act for (var i = 0; i < expectedCount; i++) { - _sut.RunAsync(() => { + _sut.RunAsync(() => + { Interlocked.Increment(ref counter); countdownEvent.Signal(); }); From 7f98c96d2d76f93719a467422f44c3f79aa11bdc Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Fri, 11 Apr 2025 13:20:24 +0200 Subject: [PATCH 06/18] Updated CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89fe2773b..dfa80acc4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Features +- Drastically improved performance of scope sync when targeting Android ([#2107](https://github.com/getsentry/sentry-unity/pull/2107)) - When running on Android, Windows or Linux, the SDK now links errors and events originating on different layers (managed, native errors) via `trace ID` ([#1997](https://github.com/getsentry/sentry-unity/pull/1997), [#2089](https://github.com/getsentry/sentry-unity/pull/2089)) - The SDK now reports the game's name as part of the app context ([2083](https://github.com/getsentry/sentry-unity/pull/2083)) - The SDK now reports the active scene's name as part of the `Unity Context` ([2084](https://github.com/getsentry/sentry-unity/pull/2084)) From f164cc6b2ef24d8240945de3b7cfdec82f4dd1a8 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Mon, 14 Apr 2025 16:06:27 +0200 Subject: [PATCH 07/18] add a high prio op to skip the line --- src/Sentry.Unity.Android/IJniExecutor.cs | 9 --- src/Sentry.Unity.Android/JniExecutor.cs | 81 ++++++++++++++----- src/Sentry.Unity.Android/SentryJava.cs | 29 +++---- .../SentryNativeAndroid.cs | 6 +- .../TestSentryJava.cs | 7 +- 5 files changed, 84 insertions(+), 48 deletions(-) delete mode 100644 src/Sentry.Unity.Android/IJniExecutor.cs diff --git a/src/Sentry.Unity.Android/IJniExecutor.cs b/src/Sentry.Unity.Android/IJniExecutor.cs deleted file mode 100644 index f465602cd..000000000 --- a/src/Sentry.Unity.Android/IJniExecutor.cs +++ /dev/null @@ -1,9 +0,0 @@ -using System; - -namespace Sentry.Unity.Android; - -internal interface IJniExecutor : IDisposable -{ - public TResult? Run(Func jniOperation, TimeSpan? timeout = null); - public void Run(Action jniOperation, TimeSpan? timeout = null); -} diff --git a/src/Sentry.Unity.Android/JniExecutor.cs b/src/Sentry.Unity.Android/JniExecutor.cs index d7654d5ae..e67ad9a62 100644 --- a/src/Sentry.Unity.Android/JniExecutor.cs +++ b/src/Sentry.Unity.Android/JniExecutor.cs @@ -7,16 +7,23 @@ namespace Sentry.Unity.Android; -internal class JniExecutor : IJniExecutor +internal class JniExecutor { private static readonly TimeSpan DefaultTimeout = TimeSpan.FromSeconds(3); - private readonly ConcurrentQueue _operationQueue = new ConcurrentQueue(); - private readonly AutoResetEvent _queueSignal = new AutoResetEvent(false); + private JniOperation? _highPriorityOperation; + private static readonly object HighPriorityOperationLock = new(); + + private readonly ConcurrentQueue _lowPriorityQueue = new(); + private const int LowPriorityQueueCapacity = 100; + private int _lowPriorityQueueSize; + private readonly IDiagnosticLogger? _logger; - private readonly CancellationTokenSource _shutdownSource = new CancellationTokenSource(); - private Thread? _workerThread; + private readonly AutoResetEvent _workerSignal = new(false); + private readonly CancellationTokenSource _shutdownSource = new(); + + private readonly Thread? _workerThread; private bool _isDisposed; private class JniOperation @@ -42,19 +49,34 @@ private void ProcessJniQueue() { AndroidJNI.AttachCurrentThread(); - var waitHandles = new[] { _queueSignal, _shutdownSource.Token.WaitHandle }; + var waitHandles = new[] { _workerSignal, _shutdownSource.Token.WaitHandle }; while (!_isDisposed) { - var index = WaitHandle.WaitAny(waitHandles); - if (index > 0 || _isDisposed) + if (_highPriorityOperation is not null) { - break; + JniOperation? highPriorityOperation; + lock (HighPriorityOperationLock) + { + highPriorityOperation = _highPriorityOperation; + _highPriorityOperation = null; + } + + ExecuteJniOperation(highPriorityOperation); + continue; } - while (!_isDisposed && _operationQueue.TryDequeue(out var operation)) + if (_lowPriorityQueue.TryDequeue(out var lowPriorityOp)) { - ExecuteJniOperation(operation); + Interlocked.Decrement(ref _lowPriorityQueueSize); + ExecuteJniOperation(lowPriorityOp); + continue; + } + + WaitHandle.WaitAny(waitHandles); + if (_shutdownSource.IsCancellationRequested || _isDisposed) + { + break; } } @@ -107,8 +129,11 @@ private void ExecuteJniOperation(JniOperation operation) var completionSource = new TaskCompletionSource(); var operation = new JniOperation(jniOperation, completionSource); - _operationQueue.Enqueue(operation); - _queueSignal.Set(); + lock (HighPriorityOperationLock) + { + _highPriorityOperation = operation; + _workerSignal.Set(); + } try { @@ -136,8 +161,12 @@ public void Run(Action jniOperation, TimeSpan? timeout = null) var completionSource = new TaskCompletionSource(); var operation = new JniOperation(jniOperation, completionSource); - _operationQueue.Enqueue(operation); - _queueSignal.Set(); + lock (HighPriorityOperationLock) + { + _highPriorityOperation = operation; + _workerSignal.Set(); + } + try { @@ -158,8 +187,24 @@ public void Run(Action jniOperation, TimeSpan? timeout = null) public void RunAsync(Action jniOperation) { var operation = new JniOperation(jniOperation); - _operationQueue.Enqueue(operation); - _queueSignal.Set(); + if(!TryEnqueueOperation(operation)) + { + // _logger?.LogWarning("Low priority JNI operation queue is full ({0}/{1}). Operation rejected.", + // _lowPriorityQueueSize, LowPriorityQueueCapacity); + } + } + + private bool TryEnqueueOperation(JniOperation operation) + { + if (Interlocked.Increment(ref _lowPriorityQueueSize) <= LowPriorityQueueCapacity) + { + _lowPriorityQueue.Enqueue(operation); + _workerSignal.Set(); + return true; + } + + Interlocked.Decrement(ref _lowPriorityQueueSize); + return false; } public void Dispose() @@ -181,7 +226,7 @@ public void Dispose() _logger?.LogError(ex, "Error during JNI executor thread shutdown"); } - _queueSignal.Dispose(); + _workerSignal.Dispose(); _shutdownSource.Dispose(); } } diff --git a/src/Sentry.Unity.Android/SentryJava.cs b/src/Sentry.Unity.Android/SentryJava.cs index c386c1de5..c67349feb 100644 --- a/src/Sentry.Unity.Android/SentryJava.cs +++ b/src/Sentry.Unity.Android/SentryJava.cs @@ -1,14 +1,12 @@ using System; -using System.Diagnostics; using Sentry.Extensibility; using UnityEngine; -using Debug = UnityEngine.Debug; namespace Sentry.Unity.Android; internal interface ISentryJava { - public bool IsEnabled(TimeSpan timeout); + public bool? IsEnabled(TimeSpan timeout); public void Init(SentryUnityOptions options, TimeSpan timeout); public string? GetInstallationId(); public bool? CrashedLastRun(); @@ -52,22 +50,19 @@ public void WriteScope( internal class SentryJava : ISentryJava { private readonly JniExecutor? _jniExecutor; + private IDiagnosticLogger? _logger; private static AndroidJavaObject GetSentryJava() => new AndroidJavaClass("io.sentry.Sentry"); private static AndroidJavaObject GetInternalSentryJava() => new AndroidJavaClass("io.sentry.android.core.InternalSentrySdk"); public SentryJava(IDiagnosticLogger? logger) { - _jniExecutor = new JniExecutor(logger); + _logger = logger; + _jniExecutor = new JniExecutor(_logger); } - public bool IsEnabled(TimeSpan timeout) + public bool? IsEnabled(TimeSpan timeout) { - if (_jniExecutor is null) - { - return false; - } - - return _jniExecutor.Run(() => + return _jniExecutor?.Run(() => { using var sentry = GetSentryJava(); return sentry.CallStatic("isEnabled"); @@ -228,7 +223,7 @@ public void AddBreadCrumb(Breadcrumb breadcrumb) public void SetExtra(string key, string? value) { - _jniExecutor?.RunAsync(() => + _jniExecutor?.Run(() => { using var sentry = GetSentryJava(); sentry.CallStatic("setExtra", key, value); @@ -237,7 +232,7 @@ public void SetExtra(string key, string? value) public void SetTag(string key, string? value) { - _jniExecutor?.RunAsync(() => + _jniExecutor?.Run(() => { using var sentry = GetSentryJava(); sentry.CallStatic("setTag", key, value); @@ -246,7 +241,7 @@ public void SetTag(string key, string? value) public void UnsetTag(string key) { - _jniExecutor?.RunAsync(() => + _jniExecutor?.Run(() => { using var sentry = GetSentryJava(); sentry.CallStatic("removeTag", key); @@ -255,7 +250,7 @@ public void UnsetTag(string key) public void SetUser(SentryUser user) { - _jniExecutor?.RunAsync(() => + _jniExecutor?.Run(() => { AndroidJavaObject? javaUser = null; try @@ -277,7 +272,7 @@ public void SetUser(SentryUser user) public void UnsetUser() { - _jniExecutor?.RunAsync(() => + _jniExecutor?.Run(() => { using var sentry = GetSentryJava(); sentry.CallStatic("setUser", null); @@ -286,7 +281,7 @@ public void UnsetUser() public void SetTrace(SentryId traceId, SpanId spanId) { - _jniExecutor?.RunAsync(() => + _jniExecutor?.Run(() => { using var sentry = GetInternalSentryJava(); // We have to explicitly cast to `(Double?)` diff --git a/src/Sentry.Unity.Android/SentryNativeAndroid.cs b/src/Sentry.Unity.Android/SentryNativeAndroid.cs index 337260ec4..ddb2e0264 100644 --- a/src/Sentry.Unity.Android/SentryNativeAndroid.cs +++ b/src/Sentry.Unity.Android/SentryNativeAndroid.cs @@ -31,7 +31,7 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry options.DiagnosticLogger?.LogDebug("Checking whether the Android SDK is present."); - // Only overwriting if it has not been set yet (by tests) + // If it's not been set (in a test) SentryJava ??= new SentryJava(options.DiagnosticLogger); if (!SentryJava.IsSentryJavaPresent()) { @@ -44,7 +44,7 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry options.DiagnosticLogger?.LogDebug("Checking whether the Android SDK has already been initialized"); - if (SentryJava.IsEnabled(TimeSpan.FromMilliseconds(200))) + if (SentryJava.IsEnabled(TimeSpan.FromMilliseconds(200)) is true) { options.DiagnosticLogger?.LogDebug("The Android SDK is already initialized"); } @@ -57,7 +57,7 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry options.DiagnosticLogger?.LogDebug("Validating Android SDK initialization"); - if (!SentryJava.IsEnabled(TimeSpan.FromMilliseconds(200))) + if (SentryJava.IsEnabled(TimeSpan.FromMilliseconds(200)) is not true) { options.DiagnosticLogger?.LogError("Failed to initialize Android Native Support"); return; diff --git a/test/Sentry.Unity.Android.Tests/TestSentryJava.cs b/test/Sentry.Unity.Android.Tests/TestSentryJava.cs index 113079033..d862c4906 100644 --- a/test/Sentry.Unity.Android.Tests/TestSentryJava.cs +++ b/test/Sentry.Unity.Android.Tests/TestSentryJava.cs @@ -1,4 +1,5 @@ using System; +using Sentry.Extensibility; namespace Sentry.Unity.Android.Tests; @@ -10,7 +11,7 @@ internal class TestSentryJava : ISentryJava public string? InstallationId { get; set; } public bool? IsCrashedLastRun { get; set; } - public bool IsEnabled(TimeSpan timeout) => Enabled; + public bool? IsEnabled(TimeSpan timeout) => Enabled; public void Init(SentryUnityOptions options, TimeSpan timeout) { } @@ -51,4 +52,8 @@ public void SetUser(SentryUser user) { } public void UnsetUser() { } public void SetTrace(SentryId traceId, SpanId spanId) { } + public void SetLogger(IDiagnosticLogger? optionsDiagnosticLogger) + { + + } } From 637fc9bd772de818ce53333863d9895d561e7098 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Mon, 14 Apr 2025 14:08:04 +0000 Subject: [PATCH 08/18] Format code --- src/Sentry.Unity.Android/JniExecutor.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sentry.Unity.Android/JniExecutor.cs b/src/Sentry.Unity.Android/JniExecutor.cs index e67ad9a62..1530983a0 100644 --- a/src/Sentry.Unity.Android/JniExecutor.cs +++ b/src/Sentry.Unity.Android/JniExecutor.cs @@ -187,7 +187,7 @@ public void Run(Action jniOperation, TimeSpan? timeout = null) public void RunAsync(Action jniOperation) { var operation = new JniOperation(jniOperation); - if(!TryEnqueueOperation(operation)) + if (!TryEnqueueOperation(operation)) { // _logger?.LogWarning("Low priority JNI operation queue is full ({0}/{1}). Operation rejected.", // _lowPriorityQueueSize, LowPriorityQueueCapacity); From 8d26a399912476d96145d285167ac6207b681a49 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Tue, 15 Apr 2025 18:47:12 +0200 Subject: [PATCH 09/18] no need for worker thread --- src/Sentry.Unity.Android/JniExecutor.cs | 229 ++++-------------- src/Sentry.Unity.Android/SentryJava.cs | 14 +- .../SentryNativeAndroid.cs | 7 +- .../SentryUnityVersion.cs | 18 +- .../JniExecutorTests.cs | 64 +---- 5 files changed, 71 insertions(+), 261 deletions(-) rename src/{Sentry.Unity.Editor => Sentry.Unity}/SentryUnityVersion.cs (60%) diff --git a/src/Sentry.Unity.Android/JniExecutor.cs b/src/Sentry.Unity.Android/JniExecutor.cs index 1530983a0..7a0a52d0b 100644 --- a/src/Sentry.Unity.Android/JniExecutor.cs +++ b/src/Sentry.Unity.Android/JniExecutor.cs @@ -3,230 +3,93 @@ using System.Threading; using System.Threading.Tasks; using Sentry.Extensibility; +using Sentry.Unity.Integrations; using UnityEngine; namespace Sentry.Unity.Android; internal class JniExecutor { - private static readonly TimeSpan DefaultTimeout = TimeSpan.FromSeconds(3); - - private JniOperation? _highPriorityOperation; - private static readonly object HighPriorityOperationLock = new(); - - private readonly ConcurrentQueue _lowPriorityQueue = new(); - private const int LowPriorityQueueCapacity = 100; - private int _lowPriorityQueueSize; - private readonly IDiagnosticLogger? _logger; - private readonly AutoResetEvent _workerSignal = new(false); - private readonly CancellationTokenSource _shutdownSource = new(); - - private readonly Thread? _workerThread; - private bool _isDisposed; - - private class JniOperation - { - public Delegate Action { get; } - public TaskCompletionSource? CompletionSource { get; } - - public JniOperation(Delegate action, TaskCompletionSource? completionSource = null) - { - Action = action; - CompletionSource = completionSource; - } - } - public JniExecutor(IDiagnosticLogger? logger) { _logger = logger; - _workerThread = new Thread(ProcessJniQueue) { IsBackground = true, Name = "SentryJniExecutorThread" }; - _workerThread.Start(); } - private void ProcessJniQueue() + /// + /// Runs a JNI operation that returns a result, waiting for completion with a timeout + /// + public TResult? Run(Func jniOperation) { - AndroidJNI.AttachCurrentThread(); - - var waitHandles = new[] { _workerSignal, _shutdownSource.Token.WaitHandle }; - - while (!_isDisposed) + if (SentryUnityVersion.IsNewerOrEqualThan("2020")) { - if (_highPriorityOperation is not null) + if (!MainThreadData.IsMainThread()) { - JniOperation? highPriorityOperation; - lock (HighPriorityOperationLock) - { - highPriorityOperation = _highPriorityOperation; - _highPriorityOperation = null; - } - - ExecuteJniOperation(highPriorityOperation); - continue; + AndroidJNI.AttachCurrentThread(); } - if (_lowPriorityQueue.TryDequeue(out var lowPriorityOp)) + try { - Interlocked.Decrement(ref _lowPriorityQueueSize); - ExecuteJniOperation(lowPriorityOp); - continue; + return jniOperation.Invoke(); } - - WaitHandle.WaitAny(waitHandles); - if (_shutdownSource.IsCancellationRequested || _isDisposed) + finally { - break; + if (!MainThreadData.IsMainThread()) + { + AndroidJNI.DetachCurrentThread(); + } } } - - AndroidJNI.DetachCurrentThread(); - _logger?.LogDebug("JNI executor thread terminated"); - } - - private void ExecuteJniOperation(JniOperation operation) - { - try + else { - switch (operation.Action) + AndroidJNI.AttachCurrentThread(); + try { - case Action action: - action.Invoke(); - operation.CompletionSource?.TrySetResult(null); - break; - case Func func1: - var result1 = func1.Invoke(); - operation.CompletionSource?.TrySetResult(result1); - break; - case Func func2: - var result2 = func2.Invoke(); - operation.CompletionSource?.TrySetResult(result2); - break; - case Func func3: - var result3 = func3.Invoke(); - operation.CompletionSource?.TrySetResult(result3); - break; - default: - var error = new NotImplementedException($"Task type '{operation.Action.GetType()}' is not implemented in JniExecutor"); - _logger?.LogError(error, "Unsupported JNI operation type"); - operation.CompletionSource?.TrySetException(error); - break; + return jniOperation.Invoke(); } - } - catch (Exception ex) - { - _logger?.LogError(ex, "Error during JNI operation execution."); - operation.CompletionSource?.TrySetException(ex); - } - } - - /// - /// Runs a JNI operation that returns a result, waiting for completion with a timeout - /// - public TResult? Run(Func jniOperation, TimeSpan? timeout = null) - { - timeout ??= DefaultTimeout; - var completionSource = new TaskCompletionSource(); - var operation = new JniOperation(jniOperation, completionSource); - - lock (HighPriorityOperationLock) - { - _highPriorityOperation = operation; - _workerSignal.Set(); - } - - try - { - if (completionSource.Task.Wait(timeout.Value)) + finally { - return (TResult?)completionSource.Task.Result; + AndroidJNI.DetachCurrentThread(); } - - _logger?.LogError("JNI operation timed out after {0}ms", timeout.Value.TotalMilliseconds); - return default; - } - catch (Exception ex) - { - _logger?.LogError(ex, "Error waiting for JNI operation result"); - return default; } } /// /// Runs a JNI operation with no return value, waiting for completion with a timeout /// - public void Run(Action jniOperation, TimeSpan? timeout = null) + public void Run(Action jniOperation) { - timeout ??= DefaultTimeout; - var completionSource = new TaskCompletionSource(); - var operation = new JniOperation(jniOperation, completionSource); - - lock (HighPriorityOperationLock) + if (SentryUnityVersion.IsNewerOrEqualThan("2020")) { - _highPriorityOperation = operation; - _workerSignal.Set(); - } - - - try - { - if (!completionSource.Task.Wait(timeout.Value)) + if (!MainThreadData.IsMainThread()) { - _logger?.LogError("JNI operation timed out after {0}ms", timeout.Value.TotalMilliseconds); + AndroidJNI.AttachCurrentThread(); } - } - catch (Exception ex) - { - _logger?.LogError(ex, "Error waiting for JNI operation completion"); - } - } - /// - /// Runs a JNI operation without waiting for completion - true fire and forget - /// - public void RunAsync(Action jniOperation) - { - var operation = new JniOperation(jniOperation); - if (!TryEnqueueOperation(operation)) - { - // _logger?.LogWarning("Low priority JNI operation queue is full ({0}/{1}). Operation rejected.", - // _lowPriorityQueueSize, LowPriorityQueueCapacity); - } - } - - private bool TryEnqueueOperation(JniOperation operation) - { - if (Interlocked.Increment(ref _lowPriorityQueueSize) <= LowPriorityQueueCapacity) - { - _lowPriorityQueue.Enqueue(operation); - _workerSignal.Set(); - return true; - } - - Interlocked.Decrement(ref _lowPriorityQueueSize); - return false; - } - - public void Dispose() - { - if (_isDisposed) - { - return; - } - - _isDisposed = true; - _shutdownSource.Cancel(); - - try - { - _workerThread?.Join(100); + try + { + jniOperation.Invoke(); + } + finally + { + if (!MainThreadData.IsMainThread()) + { + AndroidJNI.DetachCurrentThread(); + } + } } - catch (Exception ex) + else { - _logger?.LogError(ex, "Error during JNI executor thread shutdown"); + AndroidJNI.AttachCurrentThread(); + try + { + jniOperation.Invoke(); + } + finally + { + AndroidJNI.DetachCurrentThread(); + } } - - _workerSignal.Dispose(); - _shutdownSource.Dispose(); } } diff --git a/src/Sentry.Unity.Android/SentryJava.cs b/src/Sentry.Unity.Android/SentryJava.cs index c67349feb..02a61c7c5 100644 --- a/src/Sentry.Unity.Android/SentryJava.cs +++ b/src/Sentry.Unity.Android/SentryJava.cs @@ -6,8 +6,8 @@ namespace Sentry.Unity.Android; internal interface ISentryJava { - public bool? IsEnabled(TimeSpan timeout); - public void Init(SentryUnityOptions options, TimeSpan timeout); + public bool? IsEnabled(); + public void Init(SentryUnityOptions options); public string? GetInstallationId(); public bool? CrashedLastRun(); public void Close(); @@ -60,16 +60,16 @@ public SentryJava(IDiagnosticLogger? logger) _jniExecutor = new JniExecutor(_logger); } - public bool? IsEnabled(TimeSpan timeout) + public bool? IsEnabled() { return _jniExecutor?.Run(() => { using var sentry = GetSentryJava(); return sentry.CallStatic("isEnabled"); - }, timeout); + }); } - public void Init(SentryUnityOptions options, TimeSpan timeout) + public void Init(SentryUnityOptions options) { _jniExecutor?.Run(() => { @@ -111,7 +111,7 @@ public void Init(SentryUnityOptions options, TimeSpan timeout) androidOptions.Call("setAnrEnabled", false); androidOptions.Call("setEnableScopePersistence", false); }, options.DiagnosticLogger)); - }, timeout); + }); } public string? GetInstallationId() @@ -208,7 +208,7 @@ public bool IsSentryJavaPresent() public void AddBreadCrumb(Breadcrumb breadcrumb) { - _jniExecutor?.RunAsync(() => + _jniExecutor?.Run(() => { using var sentry = GetSentryJava(); using var javaBreadcrumb = new AndroidJavaObject("io.sentry.Breadcrumb"); diff --git a/src/Sentry.Unity.Android/SentryNativeAndroid.cs b/src/Sentry.Unity.Android/SentryNativeAndroid.cs index ddb2e0264..c0566227d 100644 --- a/src/Sentry.Unity.Android/SentryNativeAndroid.cs +++ b/src/Sentry.Unity.Android/SentryNativeAndroid.cs @@ -44,7 +44,7 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry options.DiagnosticLogger?.LogDebug("Checking whether the Android SDK has already been initialized"); - if (SentryJava.IsEnabled(TimeSpan.FromMilliseconds(200)) is true) + if (SentryJava.IsEnabled() is true) { options.DiagnosticLogger?.LogDebug("The Android SDK is already initialized"); } @@ -52,12 +52,11 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry { options.DiagnosticLogger?.LogInfo("Initializing the Android SDK"); - // Local testing had Init at an average of about 25ms. - SentryJava.Init(options, TimeSpan.FromMilliseconds(200)); + SentryJava.Init(options); options.DiagnosticLogger?.LogDebug("Validating Android SDK initialization"); - if (SentryJava.IsEnabled(TimeSpan.FromMilliseconds(200)) is not true) + if (SentryJava.IsEnabled() is not true) { options.DiagnosticLogger?.LogError("Failed to initialize Android Native Support"); return; diff --git a/src/Sentry.Unity.Editor/SentryUnityVersion.cs b/src/Sentry.Unity/SentryUnityVersion.cs similarity index 60% rename from src/Sentry.Unity.Editor/SentryUnityVersion.cs rename to src/Sentry.Unity/SentryUnityVersion.cs index 896abb414..dd6c928fd 100644 --- a/src/Sentry.Unity.Editor/SentryUnityVersion.cs +++ b/src/Sentry.Unity/SentryUnityVersion.cs @@ -2,19 +2,29 @@ using System.Text.RegularExpressions; using Sentry.Unity.Integrations; -namespace Sentry.Unity.Editor; +namespace Sentry.Unity; internal static class SentryUnityVersion { + private static IApplication? Application; + private static Version? Version; + public static bool IsNewerOrEqualThan(string version, IApplication? application = null) => GetVersion(application) >= new Version(version); internal static Version? GetVersion(IApplication? application = null) { - application ??= ApplicationAdapter.Instance; + if (Version is not null) + { + return Version; + } + + Application ??= application ?? ApplicationAdapter.Instance; + // The Unity version format looks like this: '2019.4.38f1', '2022.1.0a17' or '2022.1.1b4', // but Version() expects only the numerical parts, e.g. `2021.1.0` - var unityVersion = Regex.Replace(application.UnityVersion, "^([0-9]+\\.[0-9]+\\.[0-9]+)[a-z].*$", "$1"); - return new Version(unityVersion); + var unityVersion = Regex.Replace(Application.UnityVersion, "^([0-9]+\\.[0-9]+\\.[0-9]+)[a-z].*$", "$1"); + Version = new Version(unityVersion); + return Version; } } diff --git a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs index d88d38fe2..61af4b338 100644 --- a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs +++ b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs @@ -20,12 +20,6 @@ public void SetUp() _sut = new JniExecutor(_logger); } - [TearDown] - public void TearDown() - { - _sut.Dispose(); - } - [Test] public void Run_Action_ExecutesSuccessfully() { @@ -75,7 +69,7 @@ public void Run_WithTimeout_LogsErrorOnTimeout() var timeout = TimeSpan.FromMilliseconds(50); // Act - _sut.Run(slowAction, timeout); + _sut.Run(slowAction); // Assert Assert.IsTrue(_logger.Logs.Any(log => @@ -111,61 +105,5 @@ public void Run_Generic_ReturnsDefaultOnException() // Assert Assert.That(result, Is.Null); } - - [Test] - public void RunAsync_Action_DoesNotWaitForCompletion() - { - // Arrange - var executed = false; - var completionSource = new TaskCompletionSource(); - var fakeLongOperation = TimeSpan.FromMilliseconds(100); - - void Action() - { - Thread.Sleep(fakeLongOperation); // Simulate long-running operation - executed = true; - completionSource.SetResult(true); - } - - // Act - var stopwatch = new System.Diagnostics.Stopwatch(); - stopwatch.Start(); - _sut.RunAsync(Action); - stopwatch.Stop(); - - // Assert - Assert.That(stopwatch.Elapsed, Is.LessThan(fakeLongOperation), "RunAsync should return immediately"); - Assert.That(executed, Is.False, "Operation should not have completed yet"); - - // Wait for fake long work to complete - Assert.That(completionSource.Task.Wait(500), Is.True, "Operation should complete eventually"); - Assert.That(executed, Is.True, "Operation should have completed after waiting"); - } - - [Test] - public void RunAsync_MultipleOperations_AllExecuteEventually() - { - // Arrange - var counter = 0; - const int expectedCount = 5; - var countdownEvent = new CountdownEvent(expectedCount); - - // Act - for (var i = 0; i < expectedCount; i++) - { - _sut.RunAsync(() => - { - Interlocked.Increment(ref counter); - countdownEvent.Signal(); - }); - } - - // Wait with timeout instead of arbitrary sleep - var allOperationsCompleted = countdownEvent.Wait(TimeSpan.FromMilliseconds(500)); - - // Assert - Assert.That(allOperationsCompleted, Is.True, "All operations should complete within the timeout"); - Assert.That(counter, Is.EqualTo(expectedCount)); - } } } From f4c1185a358f5b21f003d87ff4cde32448db63f4 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Tue, 15 Apr 2025 19:06:15 +0200 Subject: [PATCH 10/18] unblock linux sdk build --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 49c323c6b..4e64d2d17 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,7 +34,7 @@ jobs: - target: Linux # Build using older Linux version to preserve sdk compatibility with old GLIBC # See discussion in https://github.com/getsentry/sentry-unity/issues/1730 for more details - host: ubuntu-20.04 + host: ubuntu-22.04 - target: Windows host: windows-latest uses: ./.github/workflows/sdk.yml From 0db1a93f02c9e3140cca145cf0269c390680142e Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Tue, 15 Apr 2025 19:11:00 +0200 Subject: [PATCH 11/18] fixed test --- test/Sentry.Unity.Android.Tests/TestSentryJava.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/Sentry.Unity.Android.Tests/TestSentryJava.cs b/test/Sentry.Unity.Android.Tests/TestSentryJava.cs index d862c4906..1ba016ec2 100644 --- a/test/Sentry.Unity.Android.Tests/TestSentryJava.cs +++ b/test/Sentry.Unity.Android.Tests/TestSentryJava.cs @@ -11,9 +11,9 @@ internal class TestSentryJava : ISentryJava public string? InstallationId { get; set; } public bool? IsCrashedLastRun { get; set; } - public bool? IsEnabled(TimeSpan timeout) => Enabled; + public bool? IsEnabled() => Enabled; - public void Init(SentryUnityOptions options, TimeSpan timeout) { } + public void Init(SentryUnityOptions options) { } public string? GetInstallationId() => InstallationId; @@ -52,8 +52,4 @@ public void SetUser(SentryUser user) { } public void UnsetUser() { } public void SetTrace(SentryId traceId, SpanId spanId) { } - public void SetLogger(IDiagnosticLogger? optionsDiagnosticLogger) - { - - } } From 4c7fc25c5d78c2920228636b769d01407ebbabc0 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Tue, 15 Apr 2025 21:16:05 +0200 Subject: [PATCH 12/18] . --- src/Sentry.Unity.Android/IAndroidJNI.cs | 18 +++ src/Sentry.Unity.Android/JniExecutor.cs | 64 +++++--- .../SentryNativeAndroid.cs | 2 + src/Sentry.Unity/SentryUnityVersion.cs | 2 +- .../JniExecutorTests.cs | 146 +++++++++++++----- 5 files changed, 172 insertions(+), 60 deletions(-) create mode 100644 src/Sentry.Unity.Android/IAndroidJNI.cs diff --git a/src/Sentry.Unity.Android/IAndroidJNI.cs b/src/Sentry.Unity.Android/IAndroidJNI.cs new file mode 100644 index 000000000..e18e74478 --- /dev/null +++ b/src/Sentry.Unity.Android/IAndroidJNI.cs @@ -0,0 +1,18 @@ +using UnityEngine; + +namespace Sentry.Unity.Android; + +public interface IAndroidJNI +{ + public void AttachCurrentThread(); + public void DetachCurrentThread(); +} + +public class AndroidJNIAdapter : IAndroidJNI +{ + public static readonly AndroidJNIAdapter Instance = new(); + + public void AttachCurrentThread() => AndroidJNI.AttachCurrentThread(); + + public void DetachCurrentThread() => AndroidJNI.DetachCurrentThread(); +} diff --git a/src/Sentry.Unity.Android/JniExecutor.cs b/src/Sentry.Unity.Android/JniExecutor.cs index 7a0a52d0b..0877e563c 100644 --- a/src/Sentry.Unity.Android/JniExecutor.cs +++ b/src/Sentry.Unity.Android/JniExecutor.cs @@ -1,56 +1,67 @@ using System; -using System.Collections.Concurrent; -using System.Threading; -using System.Threading.Tasks; using Sentry.Extensibility; using Sentry.Unity.Integrations; -using UnityEngine; namespace Sentry.Unity.Android; internal class JniExecutor { private readonly IDiagnosticLogger? _logger; + private readonly IAndroidJNI _androidJNI; + private readonly IApplication _application; - public JniExecutor(IDiagnosticLogger? logger) + public JniExecutor(IDiagnosticLogger? logger, IAndroidJNI? androidJNI = null, IApplication? application = null) { _logger = logger; + _androidJNI ??= androidJNI ?? AndroidJNIAdapter.Instance; + _application ??= application ?? ApplicationAdapter.Instance; } /// /// Runs a JNI operation that returns a result, waiting for completion with a timeout /// - public TResult? Run(Func jniOperation) + public TResult? Run(Func jniOperation, bool? isMainThread = null) { - if (SentryUnityVersion.IsNewerOrEqualThan("2020")) + isMainThread ??= MainThreadData.IsMainThread(); + + _logger?.LogDebug("Checking for version"); + if (SentryUnityVersion.IsNewerOrEqualThan("2020.3", _application)) { - if (!MainThreadData.IsMainThread()) + _logger?.LogDebug("Is newer"); + if (isMainThread is not true) { - AndroidJNI.AttachCurrentThread(); + _logger?.LogDebug("is non main thread"); + _androidJNI.AttachCurrentThread(); } try { + _logger?.LogDebug("invoking"); return jniOperation.Invoke(); } finally { - if (!MainThreadData.IsMainThread()) + _logger?.LogDebug("finally"); + if (isMainThread is not true) { - AndroidJNI.DetachCurrentThread(); + _logger?.LogDebug("still not main thread"); + _androidJNI.DetachCurrentThread(); } } } else { - AndroidJNI.AttachCurrentThread(); + _logger?.LogDebug("else part"); + _androidJNI.AttachCurrentThread(); try { + _logger?.LogDebug("invoking"); return jniOperation.Invoke(); } finally { - AndroidJNI.DetachCurrentThread(); + _logger?.LogDebug("finally"); + _androidJNI.DetachCurrentThread(); } } } @@ -58,37 +69,48 @@ public JniExecutor(IDiagnosticLogger? logger) /// /// Runs a JNI operation with no return value, waiting for completion with a timeout /// - public void Run(Action jniOperation) + public void Run(Action jniOperation, bool? isMainThread = null) { - if (SentryUnityVersion.IsNewerOrEqualThan("2020")) + isMainThread ??= MainThreadData.IsMainThread(); + + _logger?.LogDebug("Checking for version"); + if (SentryUnityVersion.IsNewerOrEqualThan("2020.3", _application)) { - if (!MainThreadData.IsMainThread()) + _logger?.LogDebug("Is newer"); + if (isMainThread is not true) { - AndroidJNI.AttachCurrentThread(); + _logger?.LogDebug("is non main thread"); + _androidJNI.AttachCurrentThread(); } try { + _logger?.LogDebug("invoking"); jniOperation.Invoke(); } finally { - if (!MainThreadData.IsMainThread()) + _logger?.LogDebug("finally"); + if (isMainThread is not true) { - AndroidJNI.DetachCurrentThread(); + _logger?.LogDebug("still not main thread"); + _androidJNI.DetachCurrentThread(); } } } else { - AndroidJNI.AttachCurrentThread(); + _logger?.LogDebug("else part"); + _androidJNI.AttachCurrentThread(); try { + _logger?.LogDebug("invoking"); jniOperation.Invoke(); } finally { - AndroidJNI.DetachCurrentThread(); + _logger?.LogDebug("finally"); + _androidJNI.DetachCurrentThread(); } } } diff --git a/src/Sentry.Unity.Android/SentryNativeAndroid.cs b/src/Sentry.Unity.Android/SentryNativeAndroid.cs index c0566227d..76ee1f7ae 100644 --- a/src/Sentry.Unity.Android/SentryNativeAndroid.cs +++ b/src/Sentry.Unity.Android/SentryNativeAndroid.cs @@ -21,6 +21,8 @@ public static class SentryNativeAndroid /// The Sentry Unity options to use. public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentryUnityInfo) { + MainThreadData.CollectData(); + options.DiagnosticLogger?.LogInfo("Attempting to configure native support via the Android SDK"); if (!options.AndroidNativeSupportEnabled) diff --git a/src/Sentry.Unity/SentryUnityVersion.cs b/src/Sentry.Unity/SentryUnityVersion.cs index dd6c928fd..48904146e 100644 --- a/src/Sentry.Unity/SentryUnityVersion.cs +++ b/src/Sentry.Unity/SentryUnityVersion.cs @@ -12,7 +12,7 @@ internal static class SentryUnityVersion public static bool IsNewerOrEqualThan(string version, IApplication? application = null) => GetVersion(application) >= new Version(version); - internal static Version? GetVersion(IApplication? application = null) + internal static Version GetVersion(IApplication? application = null) { if (Version is not null) { diff --git a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs index 61af4b338..fd9329877 100644 --- a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs +++ b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs @@ -4,6 +4,7 @@ using System.Threading.Tasks; using NUnit.Framework; using Sentry.Unity.Tests.SharedClasses; +using Sentry.Unity.Tests.Stubs; namespace Sentry.Unity.Android.Tests { @@ -11,99 +12,168 @@ namespace Sentry.Unity.Android.Tests public class JniExecutorTests { private TestLogger _logger = null!; // Set during SetUp - private JniExecutor _sut = null!; // Set during SetUp + private TestAndroidJNI _androidJni = null!; + private TestApplication _application = null!; + private JniExecutor _sut = null!; [SetUp] public void SetUp() { _logger = new TestLogger(); - _sut = new JniExecutor(_logger); + _androidJni = new TestAndroidJNI(); + _application = new TestApplication(unityVersion: "2019.4.40f1"); + _sut = new JniExecutor(_logger, _androidJni, _application); + } + + [TearDown] + public void TearDown() + { + _androidJni = null!; + _sut = null!; } [Test] - public void Run_Action_ExecutesSuccessfully() + public void Run_TResult_ExecutesOperation() { // Arrange - var executed = false; - var action = () => executed = true; + var operationExecuted = false; + var operation = () => + { + operationExecuted = true; + return true; + }; // Act - _sut.Run(action); + var result = _sut.Run(operation); // Assert - Assert.That(executed, Is.True); + Assert.IsTrue(operationExecuted); + Assert.IsTrue(result); } [Test] - public void Run_FuncBool_ReturnsExpectedResult() + public void Run_Void_ExecutesOperation() { // Arrange - var func = () => true; + var sut = new JniExecutor(_logger, _androidJni, _application); + bool operationExecuted = false; + var operation = () => + { + operationExecuted = true; + }; + // Act - var result = _sut.Run(func); + _sut.Run(operation); // Assert - Assert.That(result, Is.True); + Assert.IsTrue(operationExecuted); } [Test] - public void Run_FuncString_ReturnsExpectedResult() + [TestCase("2019.4.40f1", true, true, Description = "Unity <2020 + main thread = should attach")] + [TestCase("2019.4.40f1", false, true, Description = "Unity <2020 + non main thread = should attach")] + [TestCase("2020.3.1f1", true, false, Description = "Unity >2020 + main thread = should not attach")] + [TestCase("2020.3.1f1", false, true, Description = "Unity >2020+ + non main thread = should attach")] + public void Run_TResult_AttachesThreadBasedOnVersionAndThread(string unityVersion, bool isMainThread, bool shouldAttach) { // Arrange - const string? expected = "Hello"; - var func = () => expected; + _androidJni = new TestAndroidJNI(); + _application = new TestApplication(unityVersion: unityVersion); + _sut = new JniExecutor(_logger, _androidJni, _application); // Act - var result = _sut.Run(func); + _sut.Run(() => true, isMainThread); // Assert - Assert.AreEqual(expected, result); + Assert.AreEqual(shouldAttach, _androidJni.AttachCalled); + Assert.AreEqual(shouldAttach, _androidJni.DetachCalled); } [Test] - public void Run_WithTimeout_LogsErrorOnTimeout() + [TestCase("2019.4.40f1", true, true, Description = "Unity <2020 + main thread = should attach")] + [TestCase("2019.4.40f1", false, true, Description = "Unity <2020 + non main thread = should attach")] + [TestCase("2020.3.1f1", true, false, Description = "Unity >2020 + main thread = should not attach")] + [TestCase("2020.3.1f1", false, true, Description = "Unity >2020+ + non main thread = should attach")] + public void Run_Void_AttachesThreadBasedOnVersionAndThread(string unityVersion, bool isMainThread, bool shouldAttach) { // Arrange - var slowAction = () => Thread.Sleep(100); - var timeout = TimeSpan.FromMilliseconds(50); + _androidJni = new TestAndroidJNI(); + _application = new TestApplication(unityVersion: unityVersion); + _sut = new JniExecutor(_logger, _androidJni, _application); // Act - _sut.Run(slowAction); + _sut.Run(() => true, isMainThread); // Assert - Assert.IsTrue(_logger.Logs.Any(log => - log.logLevel == SentryLevel.Error && - log.message.Contains("JNI operation timed out after "))); + Assert.AreEqual(shouldAttach, _androidJni.AttachCalled); + Assert.AreEqual(shouldAttach, _androidJni.DetachCalled); } [Test] - public void Run_ThrowingOperation_LogsError() + public void Run_TResult_DetachesEvenOnException() { // Arrange - var exception = new Exception("Test exception"); - Action throwingAction = () => throw exception; + _application.UnityVersion = "2019.4.40f1"; - // Act - _sut.Run(throwingAction); + // Act & Assert + Assert.Throws(() => + _sut.Run(() => throw new InvalidOperationException())); - // Assert - Assert.IsTrue(_logger.Logs.Any(log => - log.logLevel == SentryLevel.Error && - log.message.Contains("Error during JNI operation execution."))); + Assert.IsTrue(_androidJni.AttachCalled); + Assert.IsTrue(_androidJni.DetachCalled); } [Test] - public void Run_Generic_ReturnsDefaultOnException() + public void Run_Void_DetachesEvenOnException() { // Arrange - Func throwingFunc = () => throw new Exception("Test exception"); + _application.UnityVersion = "2019.4.0f1"; - // Act - var result = _sut.Run(throwingFunc); + // Act & Assert + Assert.Throws(() => + _sut.Run(() => throw new InvalidOperationException())); - // Assert - Assert.That(result, Is.Null); + Assert.IsTrue(_androidJni.AttachCalled); + Assert.IsTrue(_androidJni.DetachCalled); + } + + [Test] + public void Run_TResult_UsesDefaultAndroidJNIWhenNotProvided() + { + // This test is more of a smoke test since we can't easily verify the use of the singleton + var result = _sut.Run(() => true); + Assert.IsTrue(result); + } + + [Test] + public void Run_Void_UsesDefaultAndroidJNIWhenNotProvided() + { + // This test is more of a smoke test since we can't easily verify the use of the singleton + Assert.DoesNotThrow(() => _sut.Run(() => { })); + } + } + + // Test implementations + public class TestAndroidJNI : IAndroidJNI + { + public bool AttachCalled { get; private set; } + public bool DetachCalled { get; private set; } + + public TestAndroidJNI() + { + AttachCalled = false; + DetachCalled = false; + } + + public void AttachCurrentThread() + { + AttachCalled = true; + } + + public void DetachCurrentThread() + { + DetachCalled = true; } } } From 38a85546dc5a7b8c69d8fe0aabc8517688c3712b Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Tue, 15 Apr 2025 22:14:55 +0200 Subject: [PATCH 13/18] tests --- src/Sentry.Unity/SentryUnityVersion.cs | 8 ++++---- test/Sentry.Unity.Android.Tests/JniExecutorTests.cs | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Sentry.Unity/SentryUnityVersion.cs b/src/Sentry.Unity/SentryUnityVersion.cs index 48904146e..2ec8e6cb1 100644 --- a/src/Sentry.Unity/SentryUnityVersion.cs +++ b/src/Sentry.Unity/SentryUnityVersion.cs @@ -6,8 +6,8 @@ namespace Sentry.Unity; internal static class SentryUnityVersion { - private static IApplication? Application; - private static Version? Version; + // Internal for tests + internal static Version? Version; public static bool IsNewerOrEqualThan(string version, IApplication? application = null) => GetVersion(application) >= new Version(version); @@ -19,11 +19,11 @@ internal static Version GetVersion(IApplication? application = null) return Version; } - Application ??= application ?? ApplicationAdapter.Instance; + application ??= ApplicationAdapter.Instance; // The Unity version format looks like this: '2019.4.38f1', '2022.1.0a17' or '2022.1.1b4', // but Version() expects only the numerical parts, e.g. `2021.1.0` - var unityVersion = Regex.Replace(Application.UnityVersion, "^([0-9]+\\.[0-9]+\\.[0-9]+)[a-z].*$", "$1"); + var unityVersion = Regex.Replace(application.UnityVersion, "^([0-9]+\\.[0-9]+\\.[0-9]+)[a-z].*$", "$1"); Version = new Version(unityVersion); return Version; } diff --git a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs index fd9329877..c0ce14950 100644 --- a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs +++ b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs @@ -11,7 +11,7 @@ namespace Sentry.Unity.Android.Tests [TestFixture] public class JniExecutorTests { - private TestLogger _logger = null!; // Set during SetUp + private TestLogger _logger = null!; private TestAndroidJNI _androidJni = null!; private TestApplication _application = null!; private JniExecutor _sut = null!; @@ -19,6 +19,8 @@ public class JniExecutorTests [SetUp] public void SetUp() { + // Reset the version to ensure it's not cached from previous tests or the Editor + SentryUnityVersion.Version = null; _logger = new TestLogger(); _androidJni = new TestAndroidJNI(); _application = new TestApplication(unityVersion: "2019.4.40f1"); From 9404aae859fb96ee683d347dc753fd39ee0769b3 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Tue, 15 Apr 2025 22:45:14 +0200 Subject: [PATCH 14/18] final form --- src/Sentry.Unity.Android/JniExecutor.cs | 93 ++++--------------- .../JniExecutorTests.cs | 80 +++------------- 2 files changed, 31 insertions(+), 142 deletions(-) diff --git a/src/Sentry.Unity.Android/JniExecutor.cs b/src/Sentry.Unity.Android/JniExecutor.cs index 0877e563c..fa443a75c 100644 --- a/src/Sentry.Unity.Android/JniExecutor.cs +++ b/src/Sentry.Unity.Android/JniExecutor.cs @@ -1,6 +1,5 @@ using System; using Sentry.Extensibility; -using Sentry.Unity.Integrations; namespace Sentry.Unity.Android; @@ -8,108 +7,50 @@ internal class JniExecutor { private readonly IDiagnosticLogger? _logger; private readonly IAndroidJNI _androidJNI; - private readonly IApplication _application; - public JniExecutor(IDiagnosticLogger? logger, IAndroidJNI? androidJNI = null, IApplication? application = null) + public JniExecutor(IDiagnosticLogger? logger, IAndroidJNI? androidJNI = null) { _logger = logger; _androidJNI ??= androidJNI ?? AndroidJNIAdapter.Instance; - _application ??= application ?? ApplicationAdapter.Instance; } - /// - /// Runs a JNI operation that returns a result, waiting for completion with a timeout - /// public TResult? Run(Func jniOperation, bool? isMainThread = null) { isMainThread ??= MainThreadData.IsMainThread(); - - _logger?.LogDebug("Checking for version"); - if (SentryUnityVersion.IsNewerOrEqualThan("2020.3", _application)) + if (isMainThread is not true) { - _logger?.LogDebug("Is newer"); - if (isMainThread is not true) - { - _logger?.LogDebug("is non main thread"); - _androidJNI.AttachCurrentThread(); - } + _androidJNI.AttachCurrentThread(); + } - try - { - _logger?.LogDebug("invoking"); - return jniOperation.Invoke(); - } - finally - { - _logger?.LogDebug("finally"); - if (isMainThread is not true) - { - _logger?.LogDebug("still not main thread"); - _androidJNI.DetachCurrentThread(); - } - } + try + { + return jniOperation.Invoke(); } - else + finally { - _logger?.LogDebug("else part"); - _androidJNI.AttachCurrentThread(); - try - { - _logger?.LogDebug("invoking"); - return jniOperation.Invoke(); - } - finally + if (isMainThread is not true) { - _logger?.LogDebug("finally"); _androidJNI.DetachCurrentThread(); } } } - /// - /// Runs a JNI operation with no return value, waiting for completion with a timeout - /// public void Run(Action jniOperation, bool? isMainThread = null) { isMainThread ??= MainThreadData.IsMainThread(); - - _logger?.LogDebug("Checking for version"); - if (SentryUnityVersion.IsNewerOrEqualThan("2020.3", _application)) + if (isMainThread is not true) { - _logger?.LogDebug("Is newer"); - if (isMainThread is not true) - { - _logger?.LogDebug("is non main thread"); - _androidJNI.AttachCurrentThread(); - } + _androidJNI.AttachCurrentThread(); + } - try - { - _logger?.LogDebug("invoking"); - jniOperation.Invoke(); - } - finally - { - _logger?.LogDebug("finally"); - if (isMainThread is not true) - { - _logger?.LogDebug("still not main thread"); - _androidJNI.DetachCurrentThread(); - } - } + try + { + jniOperation.Invoke(); } - else + finally { - _logger?.LogDebug("else part"); - _androidJNI.AttachCurrentThread(); - try - { - _logger?.LogDebug("invoking"); - jniOperation.Invoke(); - } - finally + if (isMainThread is not true) { - _logger?.LogDebug("finally"); _androidJNI.DetachCurrentThread(); } } diff --git a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs index c0ce14950..db958c92f 100644 --- a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs +++ b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs @@ -13,25 +13,14 @@ public class JniExecutorTests { private TestLogger _logger = null!; private TestAndroidJNI _androidJni = null!; - private TestApplication _application = null!; private JniExecutor _sut = null!; [SetUp] public void SetUp() { - // Reset the version to ensure it's not cached from previous tests or the Editor - SentryUnityVersion.Version = null; _logger = new TestLogger(); _androidJni = new TestAndroidJNI(); - _application = new TestApplication(unityVersion: "2019.4.40f1"); - _sut = new JniExecutor(_logger, _androidJni, _application); - } - - [TearDown] - public void TearDown() - { - _androidJni = null!; - _sut = null!; + _sut = new JniExecutor(_logger, _androidJni); } [Test] @@ -57,8 +46,7 @@ public void Run_TResult_ExecutesOperation() public void Run_Void_ExecutesOperation() { // Arrange - var sut = new JniExecutor(_logger, _androidJni, _application); - bool operationExecuted = false; + var operationExecuted = false; var operation = () => { operationExecuted = true; @@ -73,16 +61,13 @@ public void Run_Void_ExecutesOperation() } [Test] - [TestCase("2019.4.40f1", true, true, Description = "Unity <2020 + main thread = should attach")] - [TestCase("2019.4.40f1", false, true, Description = "Unity <2020 + non main thread = should attach")] - [TestCase("2020.3.1f1", true, false, Description = "Unity >2020 + main thread = should not attach")] - [TestCase("2020.3.1f1", false, true, Description = "Unity >2020+ + non main thread = should attach")] - public void Run_TResult_AttachesThreadBasedOnVersionAndThread(string unityVersion, bool isMainThread, bool shouldAttach) + [TestCase(true, false, Description = "main thread = should attach")] + [TestCase(false, true, Description = "non main thread = should not attach")] + public void Run_TResult_AttachesThreadBasedOnVersionAndThread(bool isMainThread, bool shouldAttach) { // Arrange _androidJni = new TestAndroidJNI(); - _application = new TestApplication(unityVersion: unityVersion); - _sut = new JniExecutor(_logger, _androidJni, _application); + _sut = new JniExecutor(_logger, _androidJni); // Act _sut.Run(() => true, isMainThread); @@ -93,16 +78,13 @@ public void Run_TResult_AttachesThreadBasedOnVersionAndThread(string unityVersio } [Test] - [TestCase("2019.4.40f1", true, true, Description = "Unity <2020 + main thread = should attach")] - [TestCase("2019.4.40f1", false, true, Description = "Unity <2020 + non main thread = should attach")] - [TestCase("2020.3.1f1", true, false, Description = "Unity >2020 + main thread = should not attach")] - [TestCase("2020.3.1f1", false, true, Description = "Unity >2020+ + non main thread = should attach")] - public void Run_Void_AttachesThreadBasedOnVersionAndThread(string unityVersion, bool isMainThread, bool shouldAttach) + [TestCase(true, false, Description = "main thread = should attach")] + [TestCase(false, true, Description = "non main thread = should not attach")] + public void Run_Void_AttachesThreadBasedOnVersionAndThread(bool isMainThread, bool shouldAttach) { // Arrange _androidJni = new TestAndroidJNI(); - _application = new TestApplication(unityVersion: unityVersion); - _sut = new JniExecutor(_logger, _androidJni, _application); + _sut = new JniExecutor(_logger, _androidJni); // Act _sut.Run(() => true, isMainThread); @@ -115,12 +97,9 @@ public void Run_Void_AttachesThreadBasedOnVersionAndThread(string unityVersion, [Test] public void Run_TResult_DetachesEvenOnException() { - // Arrange - _application.UnityVersion = "2019.4.40f1"; - // Act & Assert Assert.Throws(() => - _sut.Run(() => throw new InvalidOperationException())); + _sut.Run(() => throw new InvalidOperationException(), false)); Assert.IsTrue(_androidJni.AttachCalled); Assert.IsTrue(_androidJni.DetachCalled); @@ -129,53 +108,22 @@ public void Run_TResult_DetachesEvenOnException() [Test] public void Run_Void_DetachesEvenOnException() { - // Arrange - _application.UnityVersion = "2019.4.0f1"; - // Act & Assert Assert.Throws(() => - _sut.Run(() => throw new InvalidOperationException())); + _sut.Run(() => throw new InvalidOperationException(), false)); Assert.IsTrue(_androidJni.AttachCalled); Assert.IsTrue(_androidJni.DetachCalled); } - - [Test] - public void Run_TResult_UsesDefaultAndroidJNIWhenNotProvided() - { - // This test is more of a smoke test since we can't easily verify the use of the singleton - var result = _sut.Run(() => true); - Assert.IsTrue(result); - } - - [Test] - public void Run_Void_UsesDefaultAndroidJNIWhenNotProvided() - { - // This test is more of a smoke test since we can't easily verify the use of the singleton - Assert.DoesNotThrow(() => _sut.Run(() => { })); - } } - // Test implementations public class TestAndroidJNI : IAndroidJNI { public bool AttachCalled { get; private set; } public bool DetachCalled { get; private set; } - public TestAndroidJNI() - { - AttachCalled = false; - DetachCalled = false; - } - - public void AttachCurrentThread() - { - AttachCalled = true; - } + public void AttachCurrentThread() => AttachCalled = true; - public void DetachCurrentThread() - { - DetachCalled = true; - } + public void DetachCurrentThread() => DetachCalled = true; } } From a674da8f4dc57834b18db7d8b3030038e0ed4720 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Tue, 15 Apr 2025 20:46:55 +0000 Subject: [PATCH 15/18] Format code --- test/Sentry.Unity.Android.Tests/JniExecutorTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs index db958c92f..4cf6763d7 100644 --- a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs +++ b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs @@ -20,7 +20,7 @@ public void SetUp() { _logger = new TestLogger(); _androidJni = new TestAndroidJNI(); - _sut = new JniExecutor(_logger, _androidJni); + _sut = new JniExecutor(_logger, _androidJni); } [Test] From 1924cc94df4d3a5bcb30ab1d766461d65e7db268 Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Tue, 15 Apr 2025 23:12:05 +0200 Subject: [PATCH 16/18] reverted changes to sentryversion --- .../SentryUnityVersion.cs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) rename src/{Sentry.Unity => Sentry.Unity.Editor}/SentryUnityVersion.cs (66%) diff --git a/src/Sentry.Unity/SentryUnityVersion.cs b/src/Sentry.Unity.Editor/SentryUnityVersion.cs similarity index 66% rename from src/Sentry.Unity/SentryUnityVersion.cs rename to src/Sentry.Unity.Editor/SentryUnityVersion.cs index 2ec8e6cb1..896abb414 100644 --- a/src/Sentry.Unity/SentryUnityVersion.cs +++ b/src/Sentry.Unity.Editor/SentryUnityVersion.cs @@ -2,29 +2,19 @@ using System.Text.RegularExpressions; using Sentry.Unity.Integrations; -namespace Sentry.Unity; +namespace Sentry.Unity.Editor; internal static class SentryUnityVersion { - // Internal for tests - internal static Version? Version; - public static bool IsNewerOrEqualThan(string version, IApplication? application = null) => GetVersion(application) >= new Version(version); - internal static Version GetVersion(IApplication? application = null) + internal static Version? GetVersion(IApplication? application = null) { - if (Version is not null) - { - return Version; - } - application ??= ApplicationAdapter.Instance; - // The Unity version format looks like this: '2019.4.38f1', '2022.1.0a17' or '2022.1.1b4', // but Version() expects only the numerical parts, e.g. `2021.1.0` var unityVersion = Regex.Replace(application.UnityVersion, "^([0-9]+\\.[0-9]+\\.[0-9]+)[a-z].*$", "$1"); - Version = new Version(unityVersion); - return Version; + return new Version(unityVersion); } } From ef4832c12e7f524d75bd744a52655cab7c2b489b Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Wed, 16 Apr 2025 16:37:35 +0200 Subject: [PATCH 17/18] review --- .../AndroidOptionConfiguration.cs | 2 +- src/Sentry.Unity.Android/JniExecutor.cs | 26 +++++++++++++------ src/Sentry.Unity.Android/SentryJava.cs | 24 ++++++++--------- 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/src/Sentry.Unity.Android/AndroidOptionConfiguration.cs b/src/Sentry.Unity.Android/AndroidOptionConfiguration.cs index 6ca3e8143..794ff8f59 100644 --- a/src/Sentry.Unity.Android/AndroidOptionConfiguration.cs +++ b/src/Sentry.Unity.Android/AndroidOptionConfiguration.cs @@ -29,7 +29,7 @@ public AndroidOptionsConfiguration(Action callback, IDiagnost } catch (Exception e) { - _logger?.LogError(e, "Error invoking {0} ’.", methodName); + _logger?.LogError(e, "Error invoking '{0}'.", methodName); } return null; } diff --git a/src/Sentry.Unity.Android/JniExecutor.cs b/src/Sentry.Unity.Android/JniExecutor.cs index fa443a75c..8049417f6 100644 --- a/src/Sentry.Unity.Android/JniExecutor.cs +++ b/src/Sentry.Unity.Android/JniExecutor.cs @@ -14,10 +14,12 @@ public JniExecutor(IDiagnosticLogger? logger, IAndroidJNI? androidJNI = null) _androidJNI ??= androidJNI ?? AndroidJNIAdapter.Instance; } - public TResult? Run(Func jniOperation, bool? isMainThread = null) + public TResult? Run(Func jniOperation) => Run(jniOperation, MainThreadData.IsMainThread()); + + // Internal for testing + internal TResult? Run(Func jniOperation, bool isMainThread) { - isMainThread ??= MainThreadData.IsMainThread(); - if (isMainThread is not true) + if (isMainThread is false) { _androidJNI.AttachCurrentThread(); } @@ -26,19 +28,27 @@ public JniExecutor(IDiagnosticLogger? logger, IAndroidJNI? androidJNI = null) { return jniOperation.Invoke(); } + catch (Exception e) + { + _logger?.LogError(e, "Failed to execute operation"); + } finally { - if (isMainThread is not true) + if (isMainThread is false) { _androidJNI.DetachCurrentThread(); } } + + return default; } - public void Run(Action jniOperation, bool? isMainThread = null) + public void Run(Action jniOperation) => Run(jniOperation, MainThreadData.IsMainThread()); + + // Internal for testing + internal void Run(Action jniOperation, bool isMainThread) { - isMainThread ??= MainThreadData.IsMainThread(); - if (isMainThread is not true) + if (isMainThread is false) { _androidJNI.AttachCurrentThread(); } @@ -49,7 +59,7 @@ public void Run(Action jniOperation, bool? isMainThread = null) } finally { - if (isMainThread is not true) + if (isMainThread is false) { _androidJNI.DetachCurrentThread(); } diff --git a/src/Sentry.Unity.Android/SentryJava.cs b/src/Sentry.Unity.Android/SentryJava.cs index 02a61c7c5..e03fd8e16 100644 --- a/src/Sentry.Unity.Android/SentryJava.cs +++ b/src/Sentry.Unity.Android/SentryJava.cs @@ -49,7 +49,7 @@ public void WriteScope( /// internal class SentryJava : ISentryJava { - private readonly JniExecutor? _jniExecutor; + private readonly JniExecutor _jniExecutor; private IDiagnosticLogger? _logger; private static AndroidJavaObject GetSentryJava() => new AndroidJavaClass("io.sentry.Sentry"); private static AndroidJavaObject GetInternalSentryJava() => new AndroidJavaClass("io.sentry.android.core.InternalSentrySdk"); @@ -71,7 +71,7 @@ public SentryJava(IDiagnosticLogger? logger) public void Init(SentryUnityOptions options) { - _jniExecutor?.Run(() => + _jniExecutor.Run(() => { using var sentry = new AndroidJavaClass("io.sentry.android.core.SentryAndroid"); using var context = new AndroidJavaClass("com.unity3d.player.UnityPlayer") @@ -116,7 +116,7 @@ public void Init(SentryUnityOptions options) public string? GetInstallationId() { - return _jniExecutor?.Run(() => + return _jniExecutor.Run(() => { using var sentry = GetSentryJava(); using var hub = sentry.CallStatic("getCurrentHub"); @@ -137,7 +137,7 @@ public void Init(SentryUnityOptions options) /// public bool? CrashedLastRun() { - return _jniExecutor?.Run(() => + return _jniExecutor.Run(() => { using var sentry = GetSentryJava(); using var jo = sentry.CallStatic("isCrashedLastRun"); @@ -171,7 +171,7 @@ public void WriteScope( bool? GpuMultiThreadedRendering, string? GpuGraphicsShaderLevel) { - _jniExecutor?.Run(() => + _jniExecutor.Run(() => { using var gpu = new AndroidJavaObject("io.sentry.protocol.Gpu"); gpu.SetIfNotNull("name", GpuName); @@ -208,7 +208,7 @@ public bool IsSentryJavaPresent() public void AddBreadCrumb(Breadcrumb breadcrumb) { - _jniExecutor?.Run(() => + _jniExecutor.Run(() => { using var sentry = GetSentryJava(); using var javaBreadcrumb = new AndroidJavaObject("io.sentry.Breadcrumb"); @@ -223,7 +223,7 @@ public void AddBreadCrumb(Breadcrumb breadcrumb) public void SetExtra(string key, string? value) { - _jniExecutor?.Run(() => + _jniExecutor.Run(() => { using var sentry = GetSentryJava(); sentry.CallStatic("setExtra", key, value); @@ -232,7 +232,7 @@ public void SetExtra(string key, string? value) public void SetTag(string key, string? value) { - _jniExecutor?.Run(() => + _jniExecutor.Run(() => { using var sentry = GetSentryJava(); sentry.CallStatic("setTag", key, value); @@ -241,7 +241,7 @@ public void SetTag(string key, string? value) public void UnsetTag(string key) { - _jniExecutor?.Run(() => + _jniExecutor.Run(() => { using var sentry = GetSentryJava(); sentry.CallStatic("removeTag", key); @@ -250,7 +250,7 @@ public void UnsetTag(string key) public void SetUser(SentryUser user) { - _jniExecutor?.Run(() => + _jniExecutor.Run(() => { AndroidJavaObject? javaUser = null; try @@ -272,7 +272,7 @@ public void SetUser(SentryUser user) public void UnsetUser() { - _jniExecutor?.Run(() => + _jniExecutor.Run(() => { using var sentry = GetSentryJava(); sentry.CallStatic("setUser", null); @@ -281,7 +281,7 @@ public void UnsetUser() public void SetTrace(SentryId traceId, SpanId spanId) { - _jniExecutor?.Run(() => + _jniExecutor.Run(() => { using var sentry = GetInternalSentryJava(); // We have to explicitly cast to `(Double?)` From e9fc4109d900d7e0b7d351f37e367e86f76b11fd Mon Sep 17 00:00:00 2001 From: bitsandfoxes Date: Thu, 17 Apr 2025 12:24:41 +0200 Subject: [PATCH 18/18] merge jniexecutor into sentryjava --- .../AndroidJavaScopeObserver.cs | 2 +- src/Sentry.Unity.Android/JniExecutor.cs | 68 ----- src/Sentry.Unity.Android/SentryJava.cs | 242 ++++++++++++++---- .../JniExecutorTests.cs | 129 ---------- .../SentryJavaTests.cs | 53 ++++ .../TestSentryJava.cs | 2 +- 6 files changed, 250 insertions(+), 246 deletions(-) delete mode 100644 src/Sentry.Unity.Android/JniExecutor.cs delete mode 100644 test/Sentry.Unity.Android.Tests/JniExecutorTests.cs create mode 100644 test/Sentry.Unity.Android.Tests/SentryJavaTests.cs diff --git a/src/Sentry.Unity.Android/AndroidJavaScopeObserver.cs b/src/Sentry.Unity.Android/AndroidJavaScopeObserver.cs index 147a01193..55e51705f 100644 --- a/src/Sentry.Unity.Android/AndroidJavaScopeObserver.cs +++ b/src/Sentry.Unity.Android/AndroidJavaScopeObserver.cs @@ -17,7 +17,7 @@ public AndroidJavaScopeObserver(SentryOptions options, ISentryJava sentryJava) : } public override void AddBreadcrumbImpl(Breadcrumb breadcrumb) => - _sentryJava.AddBreadCrumb(breadcrumb); + _sentryJava.AddBreadcrumb(breadcrumb); public override void SetExtraImpl(string key, string? value) => _sentryJava.SetExtra(key, value); diff --git a/src/Sentry.Unity.Android/JniExecutor.cs b/src/Sentry.Unity.Android/JniExecutor.cs deleted file mode 100644 index 8049417f6..000000000 --- a/src/Sentry.Unity.Android/JniExecutor.cs +++ /dev/null @@ -1,68 +0,0 @@ -using System; -using Sentry.Extensibility; - -namespace Sentry.Unity.Android; - -internal class JniExecutor -{ - private readonly IDiagnosticLogger? _logger; - private readonly IAndroidJNI _androidJNI; - - public JniExecutor(IDiagnosticLogger? logger, IAndroidJNI? androidJNI = null) - { - _logger = logger; - _androidJNI ??= androidJNI ?? AndroidJNIAdapter.Instance; - } - - public TResult? Run(Func jniOperation) => Run(jniOperation, MainThreadData.IsMainThread()); - - // Internal for testing - internal TResult? Run(Func jniOperation, bool isMainThread) - { - if (isMainThread is false) - { - _androidJNI.AttachCurrentThread(); - } - - try - { - return jniOperation.Invoke(); - } - catch (Exception e) - { - _logger?.LogError(e, "Failed to execute operation"); - } - finally - { - if (isMainThread is false) - { - _androidJNI.DetachCurrentThread(); - } - } - - return default; - } - - public void Run(Action jniOperation) => Run(jniOperation, MainThreadData.IsMainThread()); - - // Internal for testing - internal void Run(Action jniOperation, bool isMainThread) - { - if (isMainThread is false) - { - _androidJNI.AttachCurrentThread(); - } - - try - { - jniOperation.Invoke(); - } - finally - { - if (isMainThread is false) - { - _androidJNI.DetachCurrentThread(); - } - } - } -} diff --git a/src/Sentry.Unity.Android/SentryJava.cs b/src/Sentry.Unity.Android/SentryJava.cs index e03fd8e16..55378909b 100644 --- a/src/Sentry.Unity.Android/SentryJava.cs +++ b/src/Sentry.Unity.Android/SentryJava.cs @@ -30,7 +30,7 @@ public void WriteScope( public bool IsSentryJavaPresent(); // Methods for the ScopeObserver - public void AddBreadCrumb(Breadcrumb breadcrumb); + public void AddBreadcrumb(Breadcrumb breadcrumb); public void SetExtra(string key, string? value); public void SetTag(string key, string? value); public void UnsetTag(string key); @@ -49,29 +49,43 @@ public void WriteScope( /// internal class SentryJava : ISentryJava { - private readonly JniExecutor _jniExecutor; + private readonly IAndroidJNI _androidJNI; private IDiagnosticLogger? _logger; - private static AndroidJavaObject GetSentryJava() => new AndroidJavaClass("io.sentry.Sentry"); private static AndroidJavaObject GetInternalSentryJava() => new AndroidJavaClass("io.sentry.android.core.InternalSentrySdk"); + protected virtual AndroidJavaObject GetSentryJava() => new AndroidJavaClass("io.sentry.Sentry"); - public SentryJava(IDiagnosticLogger? logger) + public SentryJava(IDiagnosticLogger? logger, IAndroidJNI? androidJNI = null) { _logger = logger; - _jniExecutor = new JniExecutor(_logger); + _androidJNI ??= androidJNI ?? AndroidJNIAdapter.Instance; } public bool? IsEnabled() { - return _jniExecutor?.Run(() => + HandleJniThreadAttachment(); + + try { using var sentry = GetSentryJava(); return sentry.CallStatic("isEnabled"); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.IsEnabled' failed."); + } + finally + { + HandleJniThreadDetachment(); + } + + return null; } public void Init(SentryUnityOptions options) { - _jniExecutor.Run(() => + HandleJniThreadAttachment(); + + try { using var sentry = new AndroidJavaClass("io.sentry.android.core.SentryAndroid"); using var context = new AndroidJavaClass("com.unity3d.player.UnityPlayer") @@ -111,18 +125,38 @@ public void Init(SentryUnityOptions options) androidOptions.Call("setAnrEnabled", false); androidOptions.Call("setEnableScopePersistence", false); }, options.DiagnosticLogger)); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.Init' failed."); + } + finally + { + HandleJniThreadDetachment(); + } } public string? GetInstallationId() { - return _jniExecutor.Run(() => + HandleJniThreadAttachment(); + + try { using var sentry = GetSentryJava(); using var hub = sentry.CallStatic("getCurrentHub"); using var options = hub?.Call("getOptions"); return options?.Call("getDistinctId"); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.GetInstallationId' failed."); + } + finally + { + HandleJniThreadDetachment(); + } + + return null; } /// @@ -137,21 +171,43 @@ public void Init(SentryUnityOptions options) /// public bool? CrashedLastRun() { - return _jniExecutor.Run(() => + HandleJniThreadAttachment(); + + try { using var sentry = GetSentryJava(); using var jo = sentry.CallStatic("isCrashedLastRun"); return jo?.Call("booleanValue"); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.CrashedLastRun' failed."); + } + finally + { + HandleJniThreadDetachment(); + } + + return null; } public void Close() { - _jniExecutor?.Run(() => + HandleJniThreadAttachment(); + + try { using var sentry = GetSentryJava(); sentry.CallStatic("close"); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.Close' failed."); + } + finally + { + HandleJniThreadDetachment(); + } } public void WriteScope( @@ -171,7 +227,9 @@ public void WriteScope( bool? GpuMultiThreadedRendering, string? GpuGraphicsShaderLevel) { - _jniExecutor.Run(() => + HandleJniThreadAttachment(); + + try { using var gpu = new AndroidJavaObject("io.sentry.protocol.Gpu"); gpu.SetIfNotNull("name", GpuName); @@ -189,7 +247,15 @@ public void WriteScope( using var contexts = scope.Call("getContexts"); contexts.Call("setGpu", gpu); })); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.WriteScope' failed."); + } + finally + { + HandleJniThreadDetachment(); + } } public bool IsSentryJavaPresent() @@ -206,9 +272,11 @@ public bool IsSentryJavaPresent() return true; } - public void AddBreadCrumb(Breadcrumb breadcrumb) + public void AddBreadcrumb(Breadcrumb breadcrumb) { - _jniExecutor.Run(() => + HandleJniThreadAttachment(); + + try { using var sentry = GetSentryJava(); using var javaBreadcrumb = new AndroidJavaObject("io.sentry.Breadcrumb"); @@ -218,75 +286,137 @@ public void AddBreadCrumb(Breadcrumb breadcrumb) using var javaLevel = breadcrumb.Level.ToJavaSentryLevel(); javaBreadcrumb.Set("level", javaLevel); sentry.CallStatic("addBreadcrumb", javaBreadcrumb, null); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.AddBreadcrumb' failed."); + } + finally + { + HandleJniThreadDetachment(); + } } public void SetExtra(string key, string? value) { - _jniExecutor.Run(() => + HandleJniThreadAttachment(); + + try { using var sentry = GetSentryJava(); sentry.CallStatic("setExtra", key, value); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.SetExtra' failed."); + } + finally + { + HandleJniThreadDetachment(); + } } public void SetTag(string key, string? value) { - _jniExecutor.Run(() => + HandleJniThreadAttachment(); + + try { using var sentry = GetSentryJava(); sentry.CallStatic("setTag", key, value); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.SetTag' failed."); + } + finally + { + HandleJniThreadDetachment(); + } } public void UnsetTag(string key) { - _jniExecutor.Run(() => + HandleJniThreadAttachment(); + + try { using var sentry = GetSentryJava(); sentry.CallStatic("removeTag", key); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.UnsetTag' failed."); + } + finally + { + HandleJniThreadDetachment(); + } } public void SetUser(SentryUser user) { - _jniExecutor.Run(() => + HandleJniThreadAttachment(); + AndroidJavaObject? javaUser = null; + + try { - AndroidJavaObject? javaUser = null; - try - { - javaUser = new AndroidJavaObject("io.sentry.protocol.User"); - javaUser.Set("email", user.Email); - javaUser.Set("id", user.Id); - javaUser.Set("username", user.Username); - javaUser.Set("ipAddress", user.IpAddress); - using var sentry = GetSentryJava(); - sentry.CallStatic("setUser", javaUser); - } - finally - { - javaUser?.Dispose(); - } - }); + javaUser = new AndroidJavaObject("io.sentry.protocol.User"); + javaUser.Set("email", user.Email); + javaUser.Set("id", user.Id); + javaUser.Set("username", user.Username); + javaUser.Set("ipAddress", user.IpAddress); + using var sentry = GetSentryJava(); + sentry.CallStatic("setUser", javaUser); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.SetUser' failed."); + } + finally + { + javaUser?.Dispose(); + HandleJniThreadDetachment(); + } } public void UnsetUser() { - _jniExecutor.Run(() => + HandleJniThreadAttachment(); + + try { using var sentry = GetSentryJava(); sentry.CallStatic("setUser", null); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.UnsetUser' failed."); + } + finally + { + HandleJniThreadDetachment(); + } } public void SetTrace(SentryId traceId, SpanId spanId) { - _jniExecutor.Run(() => + HandleJniThreadAttachment(); + + try { using var sentry = GetInternalSentryJava(); // We have to explicitly cast to `(Double?)` sentry.CallStatic("setTrace", traceId.ToString(), spanId.ToString(), (Double?)null, (Double?)null); - }); + } + catch (Exception e) + { + _logger?.LogError(e, "Calling 'SentryJava.SetTrace' failed."); + } + finally + { + HandleJniThreadDetachment(); + } } // https://github.com/getsentry/sentry-java/blob/db4dfc92f202b1cefc48d019fdabe24d487db923/sentry/src/main/java/io/sentry/SentryLevel.java#L4-L9 @@ -299,6 +429,24 @@ public void SetTrace(SentryId traceId, SpanId spanId) SentryLevel.Warning => "WARNING", _ => "DEBUG" }; + + internal void HandleJniThreadAttachment(bool? isMainThread = null) + { + isMainThread ??= MainThreadData.IsMainThread(); + if (isMainThread is false) + { + _androidJNI.AttachCurrentThread(); + } + } + + internal void HandleJniThreadDetachment(bool? isMainThread = null) + { + isMainThread ??= MainThreadData.IsMainThread(); + if (isMainThread is false) + { + _androidJNI.DetachCurrentThread(); + } + } } internal static class AndroidJavaObjectExtension diff --git a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs b/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs deleted file mode 100644 index 4cf6763d7..000000000 --- a/test/Sentry.Unity.Android.Tests/JniExecutorTests.cs +++ /dev/null @@ -1,129 +0,0 @@ -using System; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; -using NUnit.Framework; -using Sentry.Unity.Tests.SharedClasses; -using Sentry.Unity.Tests.Stubs; - -namespace Sentry.Unity.Android.Tests -{ - [TestFixture] - public class JniExecutorTests - { - private TestLogger _logger = null!; - private TestAndroidJNI _androidJni = null!; - private JniExecutor _sut = null!; - - [SetUp] - public void SetUp() - { - _logger = new TestLogger(); - _androidJni = new TestAndroidJNI(); - _sut = new JniExecutor(_logger, _androidJni); - } - - [Test] - public void Run_TResult_ExecutesOperation() - { - // Arrange - var operationExecuted = false; - var operation = () => - { - operationExecuted = true; - return true; - }; - - // Act - var result = _sut.Run(operation); - - // Assert - Assert.IsTrue(operationExecuted); - Assert.IsTrue(result); - } - - [Test] - public void Run_Void_ExecutesOperation() - { - // Arrange - var operationExecuted = false; - var operation = () => - { - operationExecuted = true; - }; - - - // Act - _sut.Run(operation); - - // Assert - Assert.IsTrue(operationExecuted); - } - - [Test] - [TestCase(true, false, Description = "main thread = should attach")] - [TestCase(false, true, Description = "non main thread = should not attach")] - public void Run_TResult_AttachesThreadBasedOnVersionAndThread(bool isMainThread, bool shouldAttach) - { - // Arrange - _androidJni = new TestAndroidJNI(); - _sut = new JniExecutor(_logger, _androidJni); - - // Act - _sut.Run(() => true, isMainThread); - - // Assert - Assert.AreEqual(shouldAttach, _androidJni.AttachCalled); - Assert.AreEqual(shouldAttach, _androidJni.DetachCalled); - } - - [Test] - [TestCase(true, false, Description = "main thread = should attach")] - [TestCase(false, true, Description = "non main thread = should not attach")] - public void Run_Void_AttachesThreadBasedOnVersionAndThread(bool isMainThread, bool shouldAttach) - { - // Arrange - _androidJni = new TestAndroidJNI(); - _sut = new JniExecutor(_logger, _androidJni); - - // Act - _sut.Run(() => true, isMainThread); - - // Assert - Assert.AreEqual(shouldAttach, _androidJni.AttachCalled); - Assert.AreEqual(shouldAttach, _androidJni.DetachCalled); - } - - [Test] - public void Run_TResult_DetachesEvenOnException() - { - // Act & Assert - Assert.Throws(() => - _sut.Run(() => throw new InvalidOperationException(), false)); - - Assert.IsTrue(_androidJni.AttachCalled); - Assert.IsTrue(_androidJni.DetachCalled); - } - - [Test] - public void Run_Void_DetachesEvenOnException() - { - // Act & Assert - Assert.Throws(() => - _sut.Run(() => throw new InvalidOperationException(), false)); - - Assert.IsTrue(_androidJni.AttachCalled); - Assert.IsTrue(_androidJni.DetachCalled); - } - } - - public class TestAndroidJNI : IAndroidJNI - { - public bool AttachCalled { get; private set; } - public bool DetachCalled { get; private set; } - - public void AttachCurrentThread() => AttachCalled = true; - - public void DetachCurrentThread() => DetachCalled = true; - } -} diff --git a/test/Sentry.Unity.Android.Tests/SentryJavaTests.cs b/test/Sentry.Unity.Android.Tests/SentryJavaTests.cs new file mode 100644 index 000000000..efbf71bab --- /dev/null +++ b/test/Sentry.Unity.Android.Tests/SentryJavaTests.cs @@ -0,0 +1,53 @@ +using NUnit.Framework; +using Sentry.Unity.Tests.SharedClasses; + +namespace Sentry.Unity.Android.Tests; + +public class SentryJavaTests +{ + private TestLogger _logger = null!; + private TestAndroidJNI _androidJni = null!; + private SentryJava _sut = null!; + + [SetUp] + public void SetUp() + { + _logger = new TestLogger(); + _androidJni = new TestAndroidJNI(); + _sut = new SentryJava(_logger, _androidJni); + } + + [Test] + [TestCase(true, false, Description = "main thread = should attach")] + [TestCase(false, true, Description = "non main thread = should not attach")] + public void HandleJniThreadAttachment_AttachesIfMainThread(bool isMainThread, bool shouldAttach) + { + // Act + _sut.HandleJniThreadAttachment(isMainThread); + + // Assert + Assert.AreEqual(shouldAttach, _androidJni.AttachCalled); + } + + [Test] + [TestCase(true, false, Description = "main thread = should detach")] + [TestCase(false, true, Description = "non main thread = should not detach")] + public void HandleJniThreadDetachment_DetachesIfMainThread(bool isMainThread, bool shouldAttach) + { + // Act + _sut.HandleJniThreadDetachment(isMainThread); + + // Assert + Assert.AreEqual(shouldAttach, _androidJni.DetachCalled); + } + + internal class TestAndroidJNI : IAndroidJNI + { + public bool AttachCalled { get; private set; } + public bool DetachCalled { get; private set; } + + public void AttachCurrentThread() => AttachCalled = true; + + public void DetachCurrentThread() => DetachCalled = true; + } +} diff --git a/test/Sentry.Unity.Android.Tests/TestSentryJava.cs b/test/Sentry.Unity.Android.Tests/TestSentryJava.cs index 1ba016ec2..a5c44e1bb 100644 --- a/test/Sentry.Unity.Android.Tests/TestSentryJava.cs +++ b/test/Sentry.Unity.Android.Tests/TestSentryJava.cs @@ -40,7 +40,7 @@ public void WriteScope( { } public bool IsSentryJavaPresent() => SentryPresent; - public void AddBreadCrumb(Breadcrumb breadcrumb) { } + public void AddBreadcrumb(Breadcrumb breadcrumb) { } public void SetExtra(string key, string? value) { } public void SetTag(string key, string? value) { }