search

xamarin / Xamarin.Forms

Xamarin.Forms is no longer supported. Migrate your apps to .NET MAUI.
https://aka.ms/xamarin-upgrade
Other
5.62k stars 1.87k forks source link

Crash in Xamarin.Android with notifications #5339

Closed prahna closed 4 years ago

prahna commented 5 years ago

Description

Hi all, we're working on a XF app and we were doing some intense push notifications testing and we found out a few issues (crashes) in the Xamarin.Android code. We're using AppCenter to track errors/crashes from the field so I do not have exact reproduction steps but all I can say is that the app was receiving 1000 notifications per 1 minute and that the app crashed after each of these exceptions.

First exception:

NotificationBuilder.TimerFinished (System.String id, System.Boolean cancel, System.Boolean allowTapInNotificationCenter) System.ObjectDisposedException: Cannot access a disposed object. Object name: 'Android.App.NotificationManager'. Stack trace: JniPeerMembers.AssertSelf (Java.Interop.IJavaPeerable self) JniPeerMembers+JniInstanceMethods.InvokeVirtualVoidMethod (System.String encodedMember, Java.Interop.IJavaPeerable self, Java.Interop.JniArgumentValue* parameters) NotificationManager.Cancel (System.Int32 id) NotificationBuilder.TimerFinished (System.String id, System.Boolean cancel, System.Boolean allowTapInNotificationCenter) NotificationBuilder+<>c__DisplayClass23_1.b__0 (System.Object x) Timer+Scheduler.TimerCB (System.Object o) IThreadPoolWorkItem.ExecuteWorkItem () ThreadPoolWorkQueue.Dispatch () _ThreadPoolWaitCallback.PerformWaitCallback ()

Second exception:

NotificationBuilder.TimerFinished (System.String id, System.Boolean cancel, System.Boolean allowTapInNotificationCenter) System.ArgumentException: The key already existed in the dictionary. Stack trace: IDictionary<TKey,TValue>.Add (TKey key, TValue value) NotificationBuilder.TimerFinished (System.String id, System.Boolean cancel, System.Boolean allowTapInNotificationCenter) NotificationBuilder+<>c__DisplayClass23_1.b__0 (System.Object x) Timer+Scheduler.TimerCB (System.Object o) IThreadPoolWorkItem.ExecuteWorkItem () ThreadPoolWorkQueue.Dispatch () _ThreadPoolWaitCallback.PerformWaitCallback ()

The third exception seems to be related to the hardware back button and not notifications but still caused a crash of the app:

Enumerable.Last[TSource] (System.Collections.Generic.IEnumerable1[T] source) System.InvalidOperationException: Sequence contains no elements Stack trace: Enumerable.Last[TSource] (System.Collections.Generic.IEnumerable1[T] source) Platform.HandleBackPressed (System.Object sender, System.EventArgs e) FormsAppCompatActivity.OnBackPressed () Activity.n_OnBackPressed (System.IntPtr jnienv, System.IntPtr native__this) (wrapper dynamic-method) System.Object.14(intptr,intptr)

Expected Behavior

Fixing the problem causing the exception or at least try/catching them so that the app doesn't crash.

Actual Behavior

App crashes after any of those exceptions

Basic Information

Thank you all for the support

pauldipietro commented 5 years ago

If you find a way to reproduce it, please let us know.

prahna commented 5 years ago

As I said, we were doing stress testing for notifications. So users on phones weren't doing anything except receiving notifications. And after 100+ messages some of them got crashes, and the only info about the crashes are what's available on AppCenter that I pasted in the post above.

pauldipietro commented 5 years ago

I understand, but we probably can't do much without a reproduction/steps to reproduce the issue, to be completely transparent about it. That's why I said to please let us know if you end up finding anything out further.

RobbiewOnline commented 5 years ago

I have also seen this issue, the common stack is...

  "exception": {
    "type": "System.ObjectDisposedException",
    "message": "Cannot access a disposed object.
    Object name: 'Android.App.NotificationManager'.",
    "stackTrace": "  

at Java.Interop.JniPeerMembers.AssertSelf (Java.Interop.IJavaPeerable self) [0x00029] in <875afdbbdb3d455686dc96177a0d7582>:0 

      at Java.Interop.JniPeerMembers+JniInstanceMethods.InvokeVirtualVoidMethod (System.String encodedMember, Java.Interop.IJavaPeerable self, Java.Interop.JniArgumentValue* parameters) [0x00000] in <875afdbbdb3d455686dc96177a0d7582>:0 

      at Android.App.NotificationManager.Notify (System.Int32 id, Android.App.Notification notification) [0x00044] in <c8e26c19a7f8438f84c78933c217b5f1>:0 

      at receivers.AlarmBroadcastReceiver+<DisplayLocalNotification>d__18.MoveNext () [0x005ff] in <f051d71440d740b59be969d7b248eefc>:0 ",

    "wrapperSdkName": "appcenter.xamarin"
  }

In our logs we capture the following properties...

What's particularly interesting is that the Notification ID starts as index 1 and increments until all notifications have been scheduled. I've seen this error when I've reached index 2, 5, 6, 8 - so this implies the NotificationManager goes out of scope whilst enumerating through the messages to be scheduled.

Here is a copy of the code in my AlarmBroadcastReceiver that's responsible for scheduling these notifications...

private void DisplayLocalNotification(Context originalContext, String title, String message, StorageService.SoundTypeEnum soundTypeEnum)
{
    // #####
    // BASED ON https://docs.microsoft.com/en-us/xamarin/android/app-fundamentals/notifications/local-notifications
    // #####

    // Sound handling
    String soundFilename =  StorageService.GetSoundNameFor(soundTypeEnum);

    // Crop the message if neccessary
    Notification.BigTextStyle textStyle = new Notification.BigTextStyle();
    textStyle.BigText(message);
    int length = message.Length;
    if (length > 80)
    {
        length = 80;
    }
    textStyle.SetSummaryText(message.Substring(0, length)+(length>=80 ? "..." : ""));

    const int pendingIntentId = 0;

    // What to launch when notification clicked on
    Intent secondIntent = new Intent(originalContext, typeof(MainActivity));
    PendingIntent pendingEventForMainActivity = PendingIntent.GetActivity(Android.App.Application.Context, pendingIntentId, secondIntent, PendingIntentFlags.OneShot);

    Notification notification;

    // This is where I get a reference to the NotificationManager - it exists okay here but seems to blow up later...
    using (var notificationManager = NotificationManager.FromContext(originalContext))
    {

        // Sound stuff
        int? resourceId = convertFilenameToResourceId(soundFilename);
        Android.Net.Uri soundUri = null;
        if (resourceId.HasValue)
        {
            soundUri = Android.Net.Uri.Parse(
                "android.resource://" + originalContext.PackageName + "/raw/" + resourceId);
        }

        // see https://stackoverflow.com/questions/47750935/alarmmanager-and-notifications-in-android-8-0oreo/47753060#47753060
        // https://stackoverflow.com/questions/50477870/how-to-play-custom-sound-in-xamarin-forms-on-android-using-local-notifications?noredirect=1#comment87978962_50477870

        if (Android.OS.Build.VERSION.SdkInt < Android.OS.BuildVersionCodes.O)
        {

            // Pre-oreo doesn't seem to be having an issue...

            if (resourceId.HasValue)
            {
                var filenameExcludingExtension = soundFilename.Substring(0, soundFilename.IndexOf("."));
                soundUri = Android.Net.Uri.Parse(
                    "android.resource://" + originalContext.PackageName + "/raw/" + filenameExcludingExtension);
            }

            var notificationBuilder = new Notification.Builder(originalContext)
                                        .SetContentTitle(title)
                                        .SetContentText(message)
                                        .SetAutoCancel(true)
                                           .SetSmallIcon(Resource.Drawable.icon) // Resource.Drawable.icon
                                                      .SetDefaults(NotificationDefaults.Lights | NotificationDefaults.Vibrate) // NotificationDefaults.All
                                           .SetContentIntent(pendingEventForMainActivity)
                                                      .SetVisibility(NotificationVisibility.Public);

            //.SetCategory(Notification.CategoryEvent)
            //.SetStyle(textStyle)

            //if (soundUri != null) {
            //  notificationBuilder.SetSound(soundUri);
            //}
            try
            {
                PlatformDifferences.PlaySound(soundFilename, Logger);
            }
            catch (Exception e)
            {
                CrashHelper.HandleExceptionInBackground(true, "Problem playing sound", this, e, new Dictionary<string, string> {
                    { "filename", soundUri.ToString() }
                });
            }

            notification = notificationBuilder.Build();
        }
        else
        {

            // ... Oreo+ DOES seem to experience the issue - using Notification Channels
            var packageName = originalContext.PackageName;
            string channelName = StorageService.GetDisplayFriendlyChannelNameForSoundType(soundTypeEnum);
            var packageAndName = packageName += "." + channelName;
            string uniqueChannelId =  GenerateUniqueChannelId(packageAndName, false);

            NotificationChannel channel;
            channel = notificationManager.GetNotificationChannel(uniqueChannelId);

            // If the channel exists, but the wrong sound is associated to it, then it's deleted and re-created
            if (channel != null && (channel.Sound == null || !channel.Sound.Equals(soundUri)))
            {
                // Delete the old channel
                notificationManager.DeleteNotificationChannel(uniqueChannelId);

                // force re-creation of channel
                channel = null; 

                // Note needs a unique notification ID otherwise 
                // "If you create a new channel with this same id, the 
                // deleted channel will be un-deleted with all of the 
                // same settings it had before it was deleted."
                uniqueChannelId = GenerateUniqueChannelId(packageAndName, true);
            }

            // No channel created (or it was just deleted) so we go ahead and create it
            if (channel == null)
            {
                channel = new NotificationChannel(uniqueChannelId, channelName, NotificationImportance.High);
                channel.EnableVibration(true);
                channel.EnableLights(true);

                AudioAttributes audioAttributes = new AudioAttributes.Builder()
                    .SetContentType(AudioContentType.Sonification)
                    .SetUsage(AudioUsageKind.NotificationEvent)
                    .Build();
                channel.SetSound(soundUri, audioAttributes);

                channel.LockscreenVisibility = NotificationVisibility.Public;
                notificationManager.CreateNotificationChannel(channel);

            }
            channel?.Dispose();

            notification = new Notification.Builder(originalContext)
                                        .SetChannelId(uniqueChannelId)
                                        .SetContentTitle(title)
                                        .SetContentText(message)
                                        .SetAutoCancel(true)
                                           .SetSmallIcon(Resource.Drawable.icon) // Resource.Drawable.icon
                                           .SetContentIntent(pendingEventForMainActivity)
                                           .SetVisibility(NotificationVisibility.Public)
                                           .SetCategory(Notification.CategoryEvent)
                                            .SetStyle(textStyle)
                                           .Build();
        }

        // Keep track of the unique notificationn ID
        int notificationId = StorageService.increment(Constants.STORAGE_KEY_NOTIFICATION_COUNTER);//....getNotificationIdAndIncrementToNextId();

        // Just added to try and log error rather than crash
        try
        {
            // *** Crash was happening here - notificationManager was null ***
            notificationManager.Notify(notificationId, notification);
        }
        catch(Exception e) {
            CrashHelper.HandleExceptionInBackground(true, "Problem setting notification", this, e, new Dictionary<string, string> {
                    { "title", title },
                    { "message", message },
                    { "notificationId", ""+notificationId }
                });
        }

        try
        {
            notification.Dispose();
        } catch (Exception e)
        {
            Logger.Log(this, $"Problem disposing notification reason:{e.Message}");
        }
    }
}

I have seen this on a mix of Android versions, 23 reports with 4 users on one version of my app...

75% - 8.0.0 25% - 7.1.1

And these devices

25% - Xperia Z5 25% - Galaxy S8 25% - Galaxy J3(2017) 25% - Others

And slightly different stats with another version of my app 7 users, 41 reports ...

42.9% - 8.0.0 28.6% - 7.1.1 14.3% - 6.0.1 14.3% - Others

42.9% - Others 28.6% - Xperia Z5 14.3% - Nexus 7 (2013) 14.3% - Galaxy S8

I have since added a try/catch to try and prevent the crash, but this will just capture it as an Exception/Error in visual studio App Center instead and will of course mean the user will not see the notification until fixed.

Any idea why the notificationManager is in scope and then ends up null?

samhouts commented 5 years ago

I wonder if this is similar to #5813.

samhouts commented 5 years ago

Still need to investigate this...

jfversluis commented 4 years ago

Hi @prahna and others. Were you able to uncover anything more about this?

samhouts commented 4 years ago

Since we haven't heard from you in more than 30 days, we hope this issue is no longer affecting you. If it is, please reopen this issue and provide the requested information so that we can look into it further. Thank you!