Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Bug] [iOS] Crash When the First Toolbar Item Has an Icon and a Second Item Gets Added #6387

Closed
abduelhamit opened this issue Jun 3, 2019 · 29 comments · Fixed by #14749
Closed
Labels
e/3 🕒 3 i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often m/high impact ⬛ p/iOS 🍎 t/bug 🐛
Milestone

Comments

@abduelhamit
Copy link

Steps to Reproduce

  1. Build the reproduction project.
  2. Tap on a toolbar item.

The relevant code:

public MainPage()
{
    item0 = new ToolbarItem("Item 0", null, ClearAndAddToolbarItems)
    {
        IconImageSource = ImageSource.FromResource("CrashTest.Image_0.png")
    };
    item1 = new ToolbarItem("Item 1", null, ClearAndAddToolbarItems)
    {
        // It doesn't matter if this item has an image or not.
        //IconImageSource = ImageSource.FromResource("CrashTest.Image_1.png")
    };
    ClearAndAddToolbarItems();
    InitializeComponent();
}

private void ClearAndAddToolbarItems()
{
    ToolbarItems.Clear();
    ToolbarItems.Add(item0);
    ToolbarItems.Add(item1);
}

Expected Behavior

No crashs.

Actual Behavior

Unhandled Exception:
System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'PrimaryToolbarItem'.
  at Foundation.NSObject.get_SuperHandle () [0x00012] in /Library/Frameworks/Xamarin.iOS.framework/Versions/12.8.0.2/src/Xamarin.iOS/Foundation/NSObject2.cs:449 
  at UIKit.UIBarButtonItem.set_Image (UIKit.UIImage value) [0x00033] in /Library/Frameworks/Xamarin.iOS.framework/Versions/12.8.0.2/src/Xamarin.iOS/UIKit/UIBarButtonItem.g.cs:653 
  at Xamarin.Forms.Platform.iOS.ToolbarItemExtensions+PrimaryToolbarItem.UpdateIconAndStyle () [0x0007e] in <db147fad2aa9412c845ff0ead92dfe20>:0 
  at System.Runtime.CompilerServices.AsyncMethodBuilderCore+<>c.<ThrowAsync>b__6_0 (System.Object state) [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/12.8.0.2/src/Xamarin.iOS/mcs/class/referencesource/mscorlib/system/runtime/compilerservices/AsyncMethodBuilder.cs:1023 
  at Foundation.NSAsyncSynchronizationContextDispatcher.Apply () [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/12.8.0.2/src/Xamarin.iOS/Foundation/NSAction.cs:178 
--- End of stack trace from previous location where exception was thrown ---

  at (wrapper managed-to-native) UIKit.UIApplication.UIApplicationMain(int,string[],intptr,intptr)
  at UIKit.UIApplication.Main (System.String[] args, System.IntPtr principal, System.IntPtr delegate) [0x00005] in /Library/Frameworks/Xamarin.iOS.framework/Versions/12.8.0.2/src/Xamarin.iOS/UIKit/UIApplication.cs:79 
  at UIKit.UIApplication.Main (System.String[] args, System.String principalClassName, System.String delegateClassName) [0x0002c] in /Library/Frameworks/Xamarin.iOS.framework/Versions/12.8.0.2/src/Xamarin.iOS/UIKit/UIApplication.cs:63 
  at CrashTest.iOS.Application.Main (System.String[] args) [0x00001] in /Users/user/Projects/CrashTest/CrashTest.iOS/Main.cs:17

Basic Information

  • Version with issue: 4.0.0.425677
  • Last known good version: None
  • IDE: Vidual Studio 2019 for Mac
  • Platform Target Frameworks:
    • iOS: 12.2
  • Nuget Packages: Xamarin.Forms
  • Affected Devices: iOS

Reproduction Link

https://drive.google.com/file/d/14VnLQbRIGfDIYxrX6IKK5Bd3cIglrk3I/view?usp=sharing

@abduelhamit abduelhamit added s/unverified New report that has yet to be verified t/bug 🐛 labels Jun 3, 2019
@pauldipietro pauldipietro added this to New in Triage Jun 3, 2019
@samhouts samhouts added i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often p/iOS 🍎 labels Jun 3, 2019
@jfversluis
Copy link
Member

jfversluis commented Jun 13, 2019

It does seem to have something to do with the fact that at least one of the items has an icon.

This line in the error:

barItem.UpdateIconAndStyle () [0x0007e] in :0

Made me try to comment out the IconImageSource line for item0. When you do that, the code works as expected. After some more digging, I found out it is also fine if you use a ImageSource.FromFile("About.png") instead of the FromResource.

@jfversluis jfversluis added e/3 🕒 3 and removed s/unverified New report that has yet to be verified labels Jun 13, 2019
@jfversluis jfversluis moved this from New to Ready For Work in Triage Jun 13, 2019
@samhouts samhouts added this to To do in iOS Ready For Work Jun 13, 2019
@samhouts samhouts removed this from Ready For Work in Triage Jun 13, 2019
@abduelhamit
Copy link
Author

It does seem to have something to do with the fact that at least one of the items has an icon.

The first item must have an icon to trigger the exception. If the first item has not an icon, but the second one has, it works as expected without an exception.

@brcinho
Copy link

brcinho commented Dec 9, 2019

+1 to fix this. Anyone found a workaround? @jfversluis We use ImageSource.FromFile() for both toolbar items but it seems that it doesn't help. So it is related to how it is named in the title I guess.
Additionally, I can confirm that the issue is of transient nature, it doesn't happen always. If the changes in the toolbar items/icons happen during navigation between pages, it happens usually then. We use AppShell, maybe that additionally increases the chances of reproducing it.

@brcinho
Copy link

brcinho commented Dec 11, 2019

Additionally, we keep the references of toolbarItems in code behind, and we change the icons dynamically to represent the state of background sync and device connection. We add the try-catch code, but the exception can not be caught, it just crashes the app. So I would kindly ask for a bit more defensive programming, try-catch here and there in the framework, prevention of NPEs with ?. and as operator, here and there. Because we already stumbled upon several issues in xamarin framework where the uncaught exceptions in the framework completely crash our app and we can not do anything to prevent it, other than some time consuming hacks, if we are able to find them.

@brcinho
Copy link

brcinho commented Dec 16, 2019

Actually, by further investigation we think this crash is related to

CachedImageRenderer.InitImageSourceHandler();

from FFImageLoading library and issue should potentially be opened there.
We commented it out and are now waiting to see if the transient crashes will go away. So far it looks good.

@jfversluis
Copy link
Member

Great find @brcinho keep us updated on further developments

@ymebrugts
Copy link

ymebrugts commented Jul 7, 2020

I had this crash as well using the shared resources in the Xamarin.Forms project:

var image = ImageSource.FromResource("NameSpaceTo.Resources.EN.png", typeof(CustomPageContainer).GetTypeInfo().Assembly);
LanguageButton.IconImageSource = image;

@samtun
Copy link

samtun commented Jul 27, 2020

Can confirm this is related to using FFImageLoading. See the bug in their repo: luberda-molinet/FFImageLoading#1460
Though I am not sure where this bug originates.

@Tommigun1980
Copy link

Tommigun1980 commented Jul 27, 2020 via email

@samtun
Copy link

samtun commented Jul 28, 2020

I know, but on my side it crashes 100% when I do the following:

  • use CachedImageRenderer.InitImageSourceHandler(); to initialise the FFImageLoading
  • use an IconImageSource for a ToolbarItem on a ContentPage

@Tommigun1980
Copy link

Tommigun1980 commented Jul 28, 2020 via email

@Tommigun1980
Copy link

Tommigun1980 commented Jul 29, 2020

I have noticed that the crash happens 100% on the first time the menu is rendered on a clean install. Consecutive boots of the app don't show this issue. After it has crashed once, I can not reproduce the crash until I reinstall the app, after which it always happens once the first time it renders the menu.

Please help.

@AlexanderMelchers
Copy link

I also ran into this issue yesterday and, to my surprise, found that nobody seems to have reported this issue to the FFImageLoading-team yet, notwithstanding the fact that this clearly seems to be an issue to do with that library. That is, the line causing issues is CachedImageRenderer.InitImageSourceHandler(); in your AppDelegate - removing it will prevent the crash.

Since this line appears to only enable FFImageLoading on image types no directly related to FFImageLoading itself, an easy work-around would be to consistently use FFImageLoading's own controls throughout your app, and disable the aforementioned line in your AppDelegate (this should be possible, assuming you're not loading external images into native controls, such as ToolbarItems, ImageButtons, etc.).

Another work-around that allows you to leave the FFImageLoading's initialization in place is to simply put a delay on insertion of your toolbar item. If inserted a while after the page has been rendered, the app doesn't crash either. As such, it may be enough to simply insert your toolbar item on the page's OnAppearing.

To get things moving with FFImageLoading, I filed a bug report there.

@acuntex
Copy link
Contributor

acuntex commented Nov 8, 2020

Also happens with Xamarin.Forms.Nuke.

I guess it happens if you use a custom Image Source Handler and has nothing to do with FFImageLoading specifically.

@acuntex
Copy link
Contributor

acuntex commented Nov 8, 2020

This seems to be definitely a crash in Xamarin.Forms
Image = await _item.IconImageSource.GetNativeImageAsync();

Apparently the default implementation by Xamarin.Forms (https://github.com/xamarin/Xamarin.Forms/blob/f35ae07a0a8471d255f7a1ebdd51499e10e0a4cb/Xamarin.Forms.Platform.iOS/Renderers/ImageRenderer.cs) is not really async, it returns without awaiting - therefor there is no time for the control to get disposed. If the implementation, either by FFImageLoading or Xamarin.Forms.Nuke or any other implementation is really async, the toolbar control can be disposed in the meantime and then the crash happens.

@samhouts This seems to be definitely a crash bug in Xamarin.Forms and is not related to either library. I can also imagine other code parts, where there is the same bug.

Before assigning Image of PrimaryToolbarItem or SecondaryToolbarItem, there has to be a check, if the control has been disposed. (Again, this has to be everywhere, where there is Code similar to await _item.IconImageSource.GetNativeImageAsync())

@abduelhamit
Copy link
Author

@acuntex My original issue is definitely related to Xamarin.Forms. It's so minimalistic that it can't be anything else.

@acuntex
Copy link
Contributor

acuntex commented Nov 10, 2020

@abduelhamit Absolutely.
You can see here:
https://github.com/xamarin/Xamarin.Forms/blob/79cc0f49fe90a59f02aa0490072b449ccdad4a27/Xamarin.Forms.Core/StreamImageSource.cs
that Streams are loaded async (unlike simple FileImageSources that are loaded directly).

Same would happen if you would use a remote image as toolbar item.

@samhouts Any update? This is a crash bug.

@PureWeen PureWeen added this to the 5.0.1 milestone Nov 18, 2020
@PureWeen PureWeen added this to To do in v5.0.1 via automation Nov 18, 2020
@Tommigun1980
Copy link

@PureWeen When will 5.0.1 come out? Thank you.

@ramtechjoe
Copy link

ramtechjoe commented Oct 4, 2021

Any updates on this. It's been open for a long time and is a system crashing bug. I can get this to occur every time by putting the application in background and then rendering the toolbar

Does anyone have good workarounds to keep this from crashing app?

@Tommigun1980
Copy link

Any updates on this. It's been open for a long time and is a system crashing bug. I can get this to occur every time by putting the application in background and then rendering the toolbar

Does anyone have good workarounds to keep this from crashing app?

I never did and resorted to removing toolbar images. @jfversluis any chance of fixing this crash in a service release? Thanks!

@ramtechjoe
Copy link

I've found that in my situation a workaround was to recreate the ToolbarItem each time the toolbar is reloaded. Previously, I was using ToolbarItem objects in memory and based on certain conditions would add/remove buttons on navigation. This is done via a Behavior attached to the page. I modified so after calling ToolbarItems.Clear() instead of using the existing objects I clone the ToolbarItems. So far this seems to be working for me

_page.ToolbarItems.Clear();

//visibleItemNames lists the Toolbars that will be displayed based on specific conditions
foreach (var name in visibleItemNames)
{
     //_toolbarByName is a dictionary that contains the ToolbarItems 
	_page.ToolbarItems.Add(CloneToolbarItem(_toolbarByName[name]));
}


private ToolbarItem CloneToolbarItem(ToolbarItem item)
{
	var clonedItem = new ToolbarItem()
	{
		Text = item.Text, 
		Command = item.Command, 
		CommandParameter = item.CommandParameter,
		Priority = item.Priority,
		IconImageSource = item.IconImageSource
	};

	return clonedItem;
}

@scriptBoris
Copy link

ANY UPDATES???? NEWS??? HELLLOOO???

@jfversluis
Copy link
Member

@scriptBoris I understand that you might feel some frustration, I also hope you understand that this will not make us move any faster. We're working as fast as we can. See #14205 for more context.

@jfversluis
Copy link
Member

@Tommigun1980 thanks for tagging this! Adding it to our list

@Tommigun1980
Copy link

Tommigun1980 commented Oct 7, 2021

@Tommigun1980 thanks for tagging this! Adding it to our list

Thank you for working so hard on fixing the issues!

Regarding this crash issue, it only happens for me when running my app on a physical device, and only the first time after an install (when the toolbar is rendered).
It never happens for me in the simulator or on consecutive boots of my app even in a physical device. So I have to reinstall the app on a real device every time I want this bug to occur.

So if you can’t reproduce please try those steps.
Thank you again!

@MaximMikhisor
Copy link

Possible hack:

Create some marker to mark ImageResource which we are using for toobar item:

public static SomeClass
{
       public static readonly String ToolbarIconMarker= "ToolbarIconMarker";
}

Assign this marker when you create ToolbarItem:

var toolbarItem = new ToolbarItem();

toolbarItem.IconImageSource = _getIconImageSource();

if (toolbarItem.IconImageSource.AutomationId == null)
   toolbarItem.IconImageSource.AutomationId = SomeClass.ToolbarIconMarker;

Somewhere in iOS project

using Foundation;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using UIKit;
using Xamarin.Forms;
using Xamarin.Forms.Platform.iOS;

[assembly: ExportImageSourceHandler(typeof(StreamImageSource), typeof(CustomStreamImagesourceHandler ))]

namespace Somenamespace
{    
    public class CustomStreamImagesourceHandler : IImageSourceHandler, IAnimationSourceHandler
	{
		StreamImagesourceHandler _handler = new StreamImagesourceHandler();

		[Preserve(Conditional = true)]
		public LsStreamImagesourceHandler()
		{
		}

		public async Task<UIImage> LoadImageAsync(ImageSource imagesource, CancellationToken cancelationToken = default(CancellationToken), float scale = 1f)
		{
			UIImage image;

			if (imagesource.AutomationId == SomeClass.ToolbarIconMarker)
			{
                                // Pay attention, just using simple approach with CancellationToken.None
                                // Implement your solution if you need something different
				Stream stream = ((StreamImageSource)imagesource).Stream(CancellationToken.None).Result;

				image = UIImage.LoadFromData(NSData.FromStream(stream), scale);
			}
			else 
			{
				image = await _handler.LoadImageAsync(imagesource, cancelationToken, scale);
			}

                        // Hack to enable color icons in ToolBar, if you need it
			image = image.ImageWithRenderingMode(UIImageRenderingMode.AlwaysOriginal);

			return image;
		}

		public async Task<FormsCAKeyFrameAnimation> LoadImageAnimationAsync(ImageSource imagesource, CancellationToken cancelationToken = default(CancellationToken), float scale = 1)
		{
			return await _handler.LoadImageAnimationAsync(imagesource, cancelationToken, scale);
		}
	}
}

@jsuarezruiz
Copy link
Contributor

The issue will be fixed by #14749 and we will include it in SR7.

@jfversluis
Copy link
Member

If you want you could grab the NuGet as described here and let us know if this fixes this issue? That will greatly speed up the review process.

Besides verifying if this particular issue is fixed also be sure to check other scenarios in the same area to make sure that this fix doesn't accidentally has side-effects 🙂

Thanks!

@jfversluis jfversluis removed this from To do in iOS Ready For Work Oct 18, 2021
@jfversluis jfversluis removed this from To do in v5.0.1 Oct 18, 2021
@jfversluis jfversluis removed this from Issues in Progress in 5.0.0 SR6 (Planning) - Target Date Oct. 13th Oct 18, 2021
@jfversluis jfversluis moved this from To Fix to PR Needs Review in 5.0.0 SR7 (Planning) - Target Date Nov. 10th Oct 18, 2021
5.0.0 SR7 (Planning) - Target Date Nov. 10th automation moved this from PR Needs Review to Done Oct 19, 2021
@skadookkunnan
Copy link

skadookkunnan commented Mar 2, 2023

Has this issue returned in 5.0.0.2545? It looks like it! @jfversluis @jsuarezruiz
I am getting this issue in the latest Xamarin Forms 5 SDK app.

Unhandled Exception:
System.ObjectDisposedException: Cannot access a disposed object.
Object name: 'Xamarin.Forms.Platform.iOS.ToolbarItemExtensions+PrimaryToolbarItem'.
  at ObjCRuntime.ThrowHelper.ThrowObjectDisposedException (System.Object o) [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/16.2.0.5/src/Xamarin.iOS/ObjCRuntime/ThrowHelper.cs:52 
  at Foundation.NSObject.get_SuperHandle () [0x00012] in /Library/Frameworks/Xamarin.iOS.framework/Versions/16.2.0.5/src/Xamarin.iOS/Foundation/NSObject2.cs:580 
  at UIKit.UIBarButtonItem.set_Image (UIKit.UIImage value) [0x0002b] in /Library/Frameworks/Xamarin.iOS.framework/Versions/16.2.0.5/src/Xamarin.iOS/UIKit/UIBarButtonItem.g.cs:847 
  at Xamarin.Forms.Platform.iOS.ToolbarItemExtensions+PrimaryToolbarItem.UpdateIconAndStyleAsync () [0x0003e] in D:\a\1\s\Xamarin.Forms.Platform.iOS\Extensions\ToolbarItemExtensions.cs:93 
  at Xamarin.Forms.Platform.iOS.ToolbarItemExtensions+PrimaryToolbarItem.OnPropertyChanged (System.Object sender, System.ComponentModel.PropertyChangedEventArgs e) [0x000e1] in D:\a\1\s\Xamarin.Forms.Platform.iOS\Extensions\ToolbarItemExtensions.cs:77 
  at System.Runtime.CompilerServices.AsyncMethodBuilderCore+<>c.<ThrowAsync>b__7_0 (System.Object state) [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/Current/src/Xamarin.iOS/mcs/class/referencesource/mscorlib/system/runtime/compilerservices/AsyncMethodBuilder.cs:1021 
  at Foundation.NSAsyncSynchronizationContextDispatcher.Apply () [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/16.2.0.5/src/Xamarin.iOS/Foundation/NSAction.cs:178 
  at (wrapper managed-to-native) UIKit.UIApplication.xamarin_UIApplicationMain(int,string[],intptr,intptr,intptr&)
  at UIKit.UIApplication.UIApplicationMain (System.Int32 argc, System.String[] argv, System.IntPtr principalClassName, System.IntPtr delegateClassName) [0x00000] in /Library/Frameworks/Xamarin.iOS.framework/Versions/16.2.0.5/src/Xamarin.iOS/UIKit/UIApplication.cs:57 
  at UIKit.UIApplication.Main (System.String[] args, System.String principalClassName, System.String delegateClassName) [0x00013] in /Library/Frameworks/Xamarin.iOS.framework/Versions/16.2.0.5/src/Xamarin.iOS/UIKit/UIApplication.cs:82 
  at Solo.iOS.Application.Main (System.String[] a) [0x00001] in /Users/sagarskadoo/Documents/Developer/OfficeProjects/Ultradata/AzureDevOps/Solo/src/Solo.iOS/Main.cs:17 

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
e/3 🕒 3 i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often m/high impact ⬛ p/iOS 🍎 t/bug 🐛
Projects
No open projects
Development

Successfully merging a pull request may close this issue.