Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Managed layer initializes native SDK on Android #1924

Merged
merged 86 commits into from
Dec 13, 2024
Merged
Show file tree
Hide file tree
Changes from 84 commits
Commits
Show all changes
86 commits
Select commit Hold shift + click to select a range
5c04e3e
how to get it working
bitsandfoxes Nov 22, 2024
99aaac4
Merge branch 'main' into feat/ios-init-cocoa
bitsandfoxes Nov 25, 2024
913ae70
Added 'native first' options
bitsandfoxes Nov 25, 2024
a05d50e
Updated Xcode modification for new flow
bitsandfoxes Nov 25, 2024
34aeaf1
Updated NativeBridges & removed NoOp
bitsandfoxes Nov 25, 2024
3e43051
Updated setup cocoa target
bitsandfoxes Nov 25, 2024
55ba78e
fixed bridge.meta
bitsandfoxes Nov 25, 2024
03d411f
leave android be
bitsandfoxes Nov 26, 2024
18c5948
2019 meta files
bitsandfoxes Nov 26, 2024
c88e5cd
keep the framework hidden from unity
bitsandfoxes Nov 26, 2024
1c25211
revert post build processing
bitsandfoxes Nov 26, 2024
63f4de2
ignore the bridge as well
bitsandfoxes Nov 26, 2024
480b3b1
added post process to remove deprecated optoin
bitsandfoxes Nov 26, 2024
76d3c63
?
bitsandfoxes Nov 26, 2024
3817247
rev
bitsandfoxes Nov 26, 2024
7d1af1a
only on 2020 and older and only for iOS
bitsandfoxes Nov 27, 2024
fdd1f29
Format code
getsentry-bot Nov 27, 2024
968bb61
exclude editor scripts for samples
bitsandfoxes Nov 27, 2024
2baa488
Merge branch 'chore/sample-no-thumb-update' of https://github.com/get…
bitsandfoxes Nov 27, 2024
7195256
wrong editor
bitsandfoxes Nov 27, 2024
2ceb47a
Merge branch 'chore/sample-no-thumb-update' into feat/ios-init-cocoa
bitsandfoxes Nov 27, 2024
978c6f0
removed option from window
bitsandfoxes Nov 27, 2024
92dc061
setting native first in config
bitsandfoxes Nov 27, 2024
4c4aab9
setting debug settings for sample
bitsandfoxes Nov 27, 2024
550a9c7
fix native init
bitsandfoxes Nov 27, 2024
72b0b79
bring back noop bridge for default setup
bitsandfoxes Nov 27, 2024
dcee725
test
bitsandfoxes Nov 27, 2024
dcff3fd
tests
bitsandfoxes Nov 27, 2024
33451f7
Format code
getsentry-bot Nov 27, 2024
2c1b027
comment
bitsandfoxes Nov 27, 2024
5a8d209
Merge branch 'feat/ios-init-cocoa' of https://github.com/getsentry/se…
bitsandfoxes Nov 27, 2024
b508957
test tweak
bitsandfoxes Nov 27, 2024
c38b8f6
moved fixer from scripts to editor
bitsandfoxes Nov 27, 2024
ff87e96
updated 'pack'
bitsandfoxes Nov 27, 2024
4fcc328
Merge branch 'chore/sample-no-thumb-update' into feat/ios-init-cocoa
bitsandfoxes Nov 27, 2024
26fa897
Updated CHANGELOG.md
bitsandfoxes Nov 27, 2024
66eaf57
Merge branch 'main' into feat/ios-init-cocoa
bitsandfoxes Nov 27, 2024
6800e06
added success log
bitsandfoxes Nov 28, 2024
de577e5
Merge branch 'feat/ios-init-cocoa' of https://github.com/getsentry/se…
bitsandfoxes Nov 28, 2024
ae92ecd
add init
bitsandfoxes Nov 28, 2024
e3d799e
do what needs to be done
bitsandfoxes Nov 28, 2024
5536bba
Format code
getsentry-bot Nov 28, 2024
7fdd463
Merge branch 'main' into feat/ios-init-cocoa
bitsandfoxes Nov 29, 2024
fc1bb2d
renamed 'native first' to 'standalone'
bitsandfoxes Nov 29, 2024
be50be6
forgot the 'native'
bitsandfoxes Nov 29, 2024
7d4b214
using enum to specify initalization type
bitsandfoxes Dec 2, 2024
6cc5957
revert valid check
bitsandfoxes Dec 2, 2024
7d4756e
fixed fallback check and tests
bitsandfoxes Dec 2, 2024
147bfa7
Merge branch 'feat/ios-init-cocoa' into feat/android-init-android
bitsandfoxes Dec 2, 2024
b40e974
added tests and logging
bitsandfoxes Dec 2, 2024
fb6569a
finalized init
bitsandfoxes Dec 2, 2024
71f1907
merged
bitsandfoxes Dec 2, 2024
ac1a71d
made the jni calls work
bitsandfoxes Dec 4, 2024
5311705
FormatException
bitsandfoxes Dec 4, 2024
1700ab4
.
bitsandfoxes Dec 4, 2024
8205098
fixed setting options
bitsandfoxes Dec 5, 2024
c2d63bd
Merge branch 'main' into feat/android-init-android
bitsandfoxes Dec 5, 2024
19ef24d
Format code
getsentry-bot Dec 5, 2024
fef9f81
fixed diagnostic levels in tests
bitsandfoxes Dec 5, 2024
33b8e32
Merge branch 'feat/android-init-android' of https://github.com/getsen…
bitsandfoxes Dec 5, 2024
faae3f3
Updated CHANGELOG.md
bitsandfoxes Dec 5, 2024
0d19559
fixed playtime tests
bitsandfoxes Dec 5, 2024
ae24d50
logging structure and test
bitsandfoxes Dec 5, 2024
9badf25
tests
bitsandfoxes Dec 5, 2024
67456fa
copy lock
bitsandfoxes Dec 5, 2024
b5d442f
added reference
bitsandfoxes Dec 5, 2024
bab1ccc
stop copying
bitsandfoxes Dec 5, 2024
ffd8221
no
bitsandfoxes Dec 5, 2024
70f1bc9
Format code
getsentry-bot Dec 5, 2024
7c928fb
option reorder
bitsandfoxes Dec 10, 2024
1185f1c
added comment to inittype option
bitsandfoxes Dec 10, 2024
327e29b
fix: improve jni stability (#1927)
bitsandfoxes Dec 12, 2024
f245c20
logging changes
bitsandfoxes Dec 12, 2024
fa34169
xcode is not gradle
bitsandfoxes Dec 12, 2024
ae49af5
fixed timeout limitations for init
bitsandfoxes Dec 12, 2024
bb5f4c1
Merge branch 'main' into feat/android-init-android
bitsandfoxes Dec 12, 2024
0514822
test
bitsandfoxes Dec 12, 2024
453c0d2
pass on timeout
bitsandfoxes Dec 12, 2024
4cf640a
test cleanup
bitsandfoxes Dec 12, 2024
5881a7e
added JNIExecutor tests
bitsandfoxes Dec 12, 2024
e8e9652
test ref
bitsandfoxes Dec 12, 2024
7dd2cbd
need this for tests
bitsandfoxes Dec 12, 2024
4c344c9
disable jni executor tests
bitsandfoxes Dec 12, 2024
d0c72e4
fixed jni executor dispose
bitsandfoxes Dec 12, 2024
0137e98
remove test jni executor
bitsandfoxes Dec 12, 2024
4757b90
catch during disposal
bitsandfoxes Dec 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@

### API Changes

- The native layer on iOS no longer self-initializes before the Unity game starts. Instead, it accepts the options at the end of the configure call. To restore the old behavior, users can opt-in to initializing native first via `IosInitializeNativeFirst`. Note that using this option comes with the limitation of baking the options into the generated Xcode project at build-time. ([#1915](https://github.com/getsentry/sentry-unity/pull/1915))
- The native layer on mobile platforms (iOS and Android) no longer self-initializes before the Unity game starts. Previously, the SDK would use the options at build-time and bake them into the native layer. Instead, the SDK will now take the options passed into the `Configure` callback and use those to initialize the native SDKs. This allows users to modify the native SDK's options at runtime programmatically.
The initialization behaviour is controlled by `IosNativeInitializationType` and `AndroidNativeInitializationType` options. These can be set from `Runtime` (default) to `BuildTime` to restore the previous flow and bake the options into the native projects. ([#1915](https://github.com/getsentry/sentry-unity/pull/1915), [#1924](https://github.com/getsentry/sentry-unity/pull/1924))

### Fixes

- On Android, the SDK not longer freezes the game when failing to sync with the native SDK ([#1927](https://github.com/getsentry/sentry-unity/pull/1927))

### Dependencies

Expand Down
4 changes: 2 additions & 2 deletions src/Sentry.Unity.Android/IJniExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ namespace Sentry.Unity.Android;

internal interface IJniExecutor : IDisposable
{
public TResult? Run<TResult>(Func<TResult?> jniOperation);
public void Run(Action jniOperation);
public TResult? Run<TResult>(Func<TResult?> jniOperation, TimeSpan? timeout = null);
public void Run(Action jniOperation, TimeSpan? timeout = null);
}
77 changes: 62 additions & 15 deletions src/Sentry.Unity.Android/JniExecutor.cs
Original file line number Diff line number Diff line change
@@ -1,26 +1,37 @@
using System;
using System.Threading;
using System.Threading.Tasks;
using Sentry.Extensibility;
using UnityEngine;

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 readonly IDiagnosticLogger? _logger;

private Delegate _currentTask = null!; // The current task will always be set together with the task event

private TaskCompletionSource<object?>? _taskCompletionSource;

private readonly object _lock = new object();

public JniExecutor()
private bool _isDisposed;
private Thread? _workerThread;

public JniExecutor(IDiagnosticLogger? logger)
{
_logger = logger;
_taskEvent = new AutoResetEvent(false);
_shutdownSource = new CancellationTokenSource();

new Thread(DoWork) { IsBackground = true, Name = "SentryJniExecutorThread" }.Start();
_workerThread = new Thread(DoWork) { IsBackground = true, Name = "SentryJniExecutorThread" };
_workerThread.Start();
}

private void DoWork()
Expand All @@ -29,7 +40,7 @@ private void DoWork()

var waitHandles = new[] { _taskEvent, _shutdownSource.Token.WaitHandle };

while (true)
while (!_isDisposed)
{
var index = WaitHandle.WaitAny(waitHandles);
if (index > 0)
Expand All @@ -50,74 +61,110 @@ private void DoWork()
_taskCompletionSource?.SetResult(null);
break;
}
case Func<bool?> func1:
case Func<bool> func1:
{
var result = func1.Invoke();
_taskCompletionSource?.SetResult(result);
break;
}
case Func<string?> func2:
case Func<bool?> func2:
{
var result = func2.Invoke();
_taskCompletionSource?.SetResult(result);
break;
}
case Func<string?> func3:
{
var result = func3.Invoke();
_taskCompletionSource?.SetResult(result);
break;
}
default:
throw new ArgumentException("Invalid type for _currentTask.");
throw new NotImplementedException($"Task type '{_currentTask?.GetType()}' with value '{_currentTask}' is not implemented in the JniExecutor.");
}
}
catch (Exception e)
{
Debug.unityLogger.Log(LogType.Exception, UnityLogger.LogTag, $"Error during JNI execution: {e}");
_logger?.LogError(e, "Error during JNI execution.");
_taskCompletionSource?.SetException(e);
}
}

AndroidJNI.DetachCurrentThread();
}

public TResult? Run<TResult>(Func<TResult?> jniOperation)
public TResult? Run<TResult>(Func<TResult?> jniOperation, TimeSpan? timeout = null)
{
lock (_lock)
{
timeout ??= DefaultTimeout;
using var timeoutCts = new CancellationTokenSource(timeout.Value);
_taskCompletionSource = new TaskCompletionSource<object?>();
_currentTask = jniOperation;
_taskEvent.Set();

try
{
return (TResult?)_taskCompletionSource.Task.GetAwaiter().GetResult();
_taskCompletionSource.Task.Wait(timeoutCts.Token);
return (TResult?)_taskCompletionSource.Task.Result;
}
catch (OperationCanceledException)
{
_logger?.LogError("JNI execution timed out.");
return default;
}
catch (Exception e)
{
Debug.unityLogger.Log(LogType.Exception, UnityLogger.LogTag, $"Error during JNI execution: {e}");
_logger?.LogError(e, "Error during JNI execution.");
return default;
}
finally
{
_currentTask = null!;
}

return default;
}
}

public void Run(Action jniOperation)
public void Run(Action jniOperation, TimeSpan? timeout = null)
{
lock (_lock)
{
timeout ??= DefaultTimeout;
using var timeoutCts = new CancellationTokenSource(timeout.Value);
_taskCompletionSource = new TaskCompletionSource<object?>();
_currentTask = jniOperation;
_taskEvent.Set();

try
{
_taskCompletionSource.Task.Wait();
_taskCompletionSource.Task.Wait(timeoutCts.Token);
}
catch (OperationCanceledException)
{
_logger?.LogError("JNI execution timed out.");
}
catch (Exception e)
{
Debug.unityLogger.Log(LogType.Exception, UnityLogger.LogTag, $"Error during JNI execution: {e}");
_logger?.LogError(e, "Error during JNI execution.");
}
finally
{
_currentTask = null!;
}
}
}

public void Dispose()
{
if (_isDisposed)
{
return;
}

_isDisposed = true;

_shutdownSource.Cancel();
_workerThread?.Join(100);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can throw for:

Exceptions
ThreadStateException
The caller attempted to join a thread that is in the Unstarted state.

ThreadInterruptedException
The thread is interrupted while waiting.

We should catch this here and suppress it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also worth reading the remarks of these APIs.

Curious why we use Thread directly instead of the Task abstraction. It's by default a background thread and it's much better to deal with cancellation, error handling etc (something for a potential future task, to change this)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why we use Thread directly instead of the Task abstraction.

The executor needs to run all the calls on a single thread. If we converted this to a task, we'd still need to ensure that so no async/await, etc. In that case, I only see risks that we do something wrong here now or in the future version of the code which would lead to the task being rescheduled to a different thread in the pool.

_taskEvent.Dispose();
}
}
105 changes: 105 additions & 0 deletions src/Sentry.Unity.Android/SentryJava.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
using System;
using System.Diagnostics;
using Sentry.Extensibility;
using UnityEngine;
using Debug = UnityEngine.Debug;

namespace Sentry.Unity.Android;

internal interface ISentryJava
{
public bool IsEnabled(IJniExecutor jniExecutor);
public bool Init(IJniExecutor jniExecutor, SentryUnityOptions options, TimeSpan timeout);
public string? GetInstallationId(IJniExecutor jniExecutor);
public bool? CrashedLastRun(IJniExecutor jniExecutor);
public void Close(IJniExecutor jniExecutor);
Expand Down Expand Up @@ -40,6 +45,93 @@ internal class SentryJava : ISentryJava
{
private static AndroidJavaObject GetSentryJava() => new AndroidJavaClass("io.sentry.Sentry");

public bool IsEnabled(IJniExecutor jniExecutor)
{
return jniExecutor.Run(() =>
{
using var sentry = GetSentryJava();
return sentry.CallStatic<bool>("isEnabled");
});
}

public bool Init(IJniExecutor jniExecutor, SentryUnityOptions options, TimeSpan timeout)
{
jniExecutor.Run(() =>
{
using var sentry = new AndroidJavaClass("io.sentry.android.core.SentryAndroid");
using var context = new AndroidJavaClass("com.unity3d.player.UnityPlayer")
.GetStatic<AndroidJavaObject>("currentActivity");

sentry.CallStatic("init", context, new AndroidOptionsConfiguration(androidOptions =>
{
androidOptions.Call("setDsn", options.Dsn);
androidOptions.Call("setDebug", options.Debug);
androidOptions.Call("setRelease", options.Release);
androidOptions.Call("setEnvironment", options.Environment);

var sentryLevelClass = new AndroidJavaClass("io.sentry.SentryLevel");
var levelString = GetLevelString(options.DiagnosticLevel);
var sentryLevel = sentryLevelClass.GetStatic<AndroidJavaObject>(levelString);
androidOptions.Call("setDiagnosticLevel", sentryLevel);

if (options.SampleRate.HasValue)
{
androidOptions.SetIfNotNull("setSampleRate", options.SampleRate.Value);
}

androidOptions.Call("setMaxBreadcrumbs", options.MaxBreadcrumbs);
androidOptions.Call("setMaxCacheItems", options.MaxCacheItems);
androidOptions.Call("setSendDefaultPii", options.SendDefaultPii);
androidOptions.Call("setEnableNdk", options.NdkIntegrationEnabled);
androidOptions.Call("setEnableScopeSync", options.NdkScopeSyncEnabled);

// Options that are not to be set by the user
// We're disabling some integrations as to not duplicate event or because the SDK relies on the .NET SDK
// implementation of certain feature - i.e. Session Tracking

// Note: doesn't work - produces a blank (white) screenshot
androidOptions.Call("setAttachScreenshot", false);
androidOptions.Call("setEnableAutoSessionTracking", false);
androidOptions.Call("setEnableActivityLifecycleBreadcrumbs", false);
androidOptions.Call("setAnrEnabled", false);
androidOptions.Call("setEnableScopePersistence", false);
}, options.DiagnosticLogger));
}, timeout);

return IsEnabled(jniExecutor);
}

internal class AndroidOptionsConfiguration : AndroidJavaProxy
{
private readonly Action<AndroidJavaObject> _callback;
private readonly IDiagnosticLogger? _logger;

public AndroidOptionsConfiguration(Action<AndroidJavaObject> 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(() =>
Expand Down Expand Up @@ -165,6 +257,17 @@ public ScopeCallback(Action<AndroidJavaObject> callback) : base("io.sentry.Scope
return null;
}
}

// https://github.com/getsentry/sentry-java/blob/db4dfc92f202b1cefc48d019fdabe24d487db923/sentry/src/main/java/io/sentry/SentryLevel.java#L4-L9
internal static string GetLevelString(SentryLevel level) => level switch
{
SentryLevel.Debug => "DEBUG",
SentryLevel.Error => "ERROR",
SentryLevel.Fatal => "FATAL",
SentryLevel.Info => "INFO",
SentryLevel.Warning => "WARNING",
_ => "DEBUG"
};
}

internal static class AndroidJavaObjectExtension
Expand All @@ -186,6 +289,8 @@ public static void SetIfNotNull<T>(this AndroidJavaObject javaObject, string pro
}
public static void SetIfNotNull(this AndroidJavaObject javaObject, string property, int? value) =>
SetIfNotNull(javaObject, property, value, "java.lang.Integer");
public static void SetIfNotNull(this AndroidJavaObject javaObject, string property, bool value) =>
SetIfNotNull(javaObject, property, value, "java.lang.Boolean");
public static void SetIfNotNull(this AndroidJavaObject javaObject, string property, bool? value) =>
SetIfNotNull(javaObject, property, value, "java.lang.Boolean");
}
17 changes: 15 additions & 2 deletions src/Sentry.Unity.Android/SentryNativeAndroid.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,18 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry
return;
}

JniExecutor ??= new JniExecutor();
JniExecutor ??= new JniExecutor(options.DiagnosticLogger);

if (SentryJava.IsEnabled(JniExecutor))
{
options.DiagnosticLogger?.LogDebug("The Android SDK is already initialized");
}
// Local testing had Init at an average of about 25ms.
else if (!SentryJava.Init(JniExecutor, options, TimeSpan.FromMilliseconds(200)))
{
options.DiagnosticLogger?.LogError("Failed to initialize Android Native Support");
return;
}

options.NativeContextWriter = new NativeContextWriter(JniExecutor, SentryJava);
options.ScopeObserver = new AndroidJavaScopeObserver(options, JniExecutor);
Expand Down Expand Up @@ -98,6 +109,8 @@ public static void Configure(SentryUnityOptions options, ISentryUnityInfo sentry
options.DiagnosticLogger?.LogDebug("Failed to create new 'Default User ID'.");
}
}

options.DiagnosticLogger?.LogInfo("Successfully configured the Android SDK");
}

/// <summary>
Expand All @@ -119,7 +132,7 @@ internal static void Close(SentryUnityOptions options, ISentryUnityInfo sentryUn

// 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();
JniExecutor ??= new JniExecutor(options.DiagnosticLogger);
SentryJava?.Close(JniExecutor);
JniExecutor.Dispose();
}
Expand Down
Loading
Loading