diff --git a/CHANGELOG.md b/CHANGELOG.md index 48a1046870..645de6ab43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +### Fixes +- Fixed JNI Error when accessing Android device data from multiple threads ([#3802](https://github.com/getsentry/sentry-dotnet/pull/3802)) + ### Dependencies - Bump CLI from v2.39.0 to v2.39.1 ([#3799](https://github.com/getsentry/sentry-dotnet/pull/3799)) diff --git a/Directory.Build.props b/Directory.Build.props index 8f7593a6e2..0ac7c083f7 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -2,7 +2,7 @@ 5.0.0-alpha.1 - 12 + 13 true true $(MSBuildThisFileDirectory).assets\Sentry.snk diff --git a/src/Sentry.Maui/Internal/MauiDeviceData.cs b/src/Sentry.Maui/Internal/MauiDeviceData.cs index 3e92772ffa..cffb3ebc11 100644 --- a/src/Sentry.Maui/Internal/MauiDeviceData.cs +++ b/src/Sentry.Maui/Internal/MauiDeviceData.cs @@ -6,7 +6,14 @@ namespace Sentry.Maui.Internal; internal static class MauiDeviceData { - public static void ApplyMauiDeviceData(this Device device, IDiagnosticLogger? logger) +#if NET9_0_OR_GREATER && ANDROID + private static readonly Lock JniLock = new(); +#elif ANDROID + private static readonly object JniLock = new(); +#endif + + public static void ApplyMauiDeviceData(this Device device, IDiagnosticLogger? logger, + INetworkStatusListener? networkStatusListener) { try { @@ -22,7 +29,17 @@ public static void ApplyMauiDeviceData(this Device device, IDiagnosticLogger? lo device.Name ??= deviceInfo.Name; device.Manufacturer ??= deviceInfo.Manufacturer; device.Model ??= deviceInfo.Model; +#if ANDROID + // DeviceInfo.Idiom is not threadsafe on Android + // See: https://github.com/getsentry/sentry-dotnet/issues/3627 + lock (JniLock) + { + device.DeviceType ??= deviceInfo.Idiom.ToString(); + } +#else device.DeviceType ??= deviceInfo.Idiom.ToString(); +#endif + device.Simulator ??= deviceInfo.DeviceType switch { DeviceType.Virtual => true, @@ -54,10 +71,12 @@ public static void ApplyMauiDeviceData(this Device device, IDiagnosticLogger? lo logger?.LogDebug("No permission to read battery state from the device."); } - // https://docs.microsoft.com/dotnet/maui/platform-integration/communication/networking#using-connectivity try { - device.IsOnline ??= Connectivity.NetworkAccess == NetworkAccess.Internet; + // Note: Connectivity.NetworkAccess is not threadsafe on Android. As we already have a network listener + // monitoring the status of the network, we get the online satus from there instead (on all platforms) + // See: https://github.com/getsentry/sentry-dotnet/issues/3627 + device.IsOnline ??= networkStatusListener?.Online; } catch (PermissionException) { @@ -79,6 +98,13 @@ public static void ApplyMauiDeviceData(this Device device, IDiagnosticLogger? lo MainThread.BeginInvokeOnMainThread(() => CaptureDisplayInfo(resetEvent)); resetEvent.Wait(); } +#elif ANDROID + // DeviceDisplay.Current is not threadsafe on Android. + // See: https://github.com/getsentry/sentry-dotnet/issues/3627 + lock (JniLock) + { + CaptureDisplayInfo(); + } #else CaptureDisplayInfo(); #endif diff --git a/src/Sentry.Maui/Internal/ScreenshotAttachment.cs b/src/Sentry.Maui/Internal/ScreenshotAttachment.cs index c867862f85..60dab1a1fa 100644 --- a/src/Sentry.Maui/Internal/ScreenshotAttachment.cs +++ b/src/Sentry.Maui/Internal/ScreenshotAttachment.cs @@ -27,6 +27,12 @@ internal class ScreenshotAttachmentContent : IAttachmentContent { private readonly SentryMauiOptions _options; +#if NET9_0_OR_GREATER && ANDROID + private static readonly Lock JniLock = new(); +#elif ANDROID + private static readonly object JniLock = new(); +#endif + public ScreenshotAttachmentContent(SentryMauiOptions options) { _options = options; @@ -36,7 +42,7 @@ public Stream GetStream() { var stream = Stream.Null; // Not including this on Windows specific build because on WinUI this can deadlock. -#if __ANDROID__ || __IOS__ +#if (__ANDROID__ || __IOS__) Stream CaptureScreenBlocking() { // This actually runs synchronously (returning Task.FromResult) on the following platforms: @@ -52,22 +58,27 @@ Stream CaptureScreenBlocking() return stream; } +#endif +#if __IOS__ if (MainThread.IsMainThread) { stream = CaptureScreenBlocking(); } else { -#if __ANDROID__ //Android does not require UI thread to capture screen but iOS does. - stream = CaptureScreenBlocking(); -#else + // Screenshots have to be captured from the UI thread on iOS stream = MainThread.InvokeOnMainThreadAsync(async () => { var screen = await Screenshot.Default.CaptureAsync().ConfigureAwait(true); return await screen.OpenReadAsync(ScreenshotFormat.Jpeg).ConfigureAwait(true); }).ConfigureAwait(false).GetAwaiter().GetResult(); -#endif + } +#elif __ANDROID__ + // Capturing screenshots is not threadsafe on Android + lock (JniLock) + { + stream = CaptureScreenBlocking(); } #endif return stream; diff --git a/src/Sentry.Maui/Internal/SentryMauiEventProcessor.cs b/src/Sentry.Maui/Internal/SentryMauiEventProcessor.cs index 0c8bceccb5..6048da72fc 100644 --- a/src/Sentry.Maui/Internal/SentryMauiEventProcessor.cs +++ b/src/Sentry.Maui/Internal/SentryMauiEventProcessor.cs @@ -17,7 +17,7 @@ public SentryEvent Process(SentryEvent @event) { @event.Sdk.Name = Constants.SdkName; @event.Sdk.Version = Constants.SdkVersion; - @event.Contexts.Device.ApplyMauiDeviceData(_options.DiagnosticLogger); + @event.Contexts.Device.ApplyMauiDeviceData(_options.DiagnosticLogger, _options.NetworkStatusListener); @event.Contexts.OperatingSystem.ApplyMauiOsData(_options.DiagnosticLogger); @event.Contexts.App.InForeground = InForeground; diff --git a/src/Sentry/Internal/Http/CachingTransport.cs b/src/Sentry/Internal/Http/CachingTransport.cs index d783d0dbd0..f6d4c9b856 100644 --- a/src/Sentry/Internal/Http/CachingTransport.cs +++ b/src/Sentry/Internal/Http/CachingTransport.cs @@ -41,7 +41,7 @@ internal class CachingTransport : ITransport, IDisposable // and write from/to the cache directory. // Lock usage is minimized by moving files that are being processed to a special directory // where collisions are not expected. - private readonly Lock _cacheDirectoryLock = new(); + private readonly SentryLock _cacheDirectoryLock = new(); private readonly CancellationTokenSource _workerCts = new(); private Task _worker = null!; diff --git a/src/Sentry/Internal/Lock.cs b/src/Sentry/Internal/SentryLock.cs similarity index 78% rename from src/Sentry/Internal/Lock.cs rename to src/Sentry/Internal/SentryLock.cs index f831c64104..6889484f6b 100644 --- a/src/Sentry/Internal/Lock.cs +++ b/src/Sentry/Internal/SentryLock.cs @@ -1,10 +1,10 @@ namespace Sentry.Internal; -internal class Lock : IDisposable +internal class SentryLock : IDisposable { private readonly Signal _signal; - public Lock() => _signal = new Signal(true); + public SentryLock() => _signal = new Signal(true); public async Task AcquireAsync(CancellationToken cancellationToken = default) { diff --git a/test/Sentry.Maui.Tests/SentryMauiScreenshotTests.cs b/test/Sentry.Maui.Tests/SentryMauiScreenshotTests.cs index e2b60f5354..601b042a1a 100644 --- a/test/Sentry.Maui.Tests/SentryMauiScreenshotTests.cs +++ b/test/Sentry.Maui.Tests/SentryMauiScreenshotTests.cs @@ -172,5 +172,40 @@ public async Task CaptureException_BeforeCaptureScreenshot_DefaultAsync() envelopeItem!.TryGetFileName().Should().Be("screenshot.jpg"); } } + + [Fact] + public async Task CaptureException_AttachScreenshot_Threadsafe() + { + // Arrange + var builder = _fixture.Builder.UseSentry(options => + { + options.AttachScreenshot = true; + }); + await using var app = builder.Build(); + var client = app.Services.GetRequiredService(); + + // Act + var tasks = new List>(); + for (var i = 0; i < 20; i++) + { + var j = i; + tasks.Add(Task.Run(() => + { + var exSample = new NotImplementedException("Sample Exception " + j); + var sentryId = client.CaptureException(exSample); + client.FlushAsync(); + return sentryId; + })); + } + + // Assert + while (tasks.Any()) + { + var finishedTask = await Task.WhenAny(tasks); + + finishedTask.Exception.Should().BeNull(); + tasks.Remove(finishedTask); + } + } #endif }