Skip to content

[shell] [windows] fix NRE when clearing shell items - #15220

Merged
PureWeen merged 2 commits into
dotnet:mainfrom
jonathanpeppers:HotReloadShell
May 22, 2023
Merged

PureWeen merged 2 commits into
dotnet:mainfrom
jonathanpeppers:HotReloadShell

Conversation

@jonathanpeppers

Copy link
Copy Markdown
Member

Fixes: #13775

You can cause MAUI to throw an NRE by doing:

  1. Debug/run, have XAML hot reload enabled

  2. Comment out the ShellContent in AppShell.xaml, hot reload

  3. Uncomment the ShellContent in AppShell.xaml, hot reload

Crashes with:

System.NullReferenceException: Object reference not set to an instance of an object.
at Microsoft.Maui.Controls.Handlers.ShellSectionHandler.SetVirtualView(IElement view)
at Microsoft.Maui.Controls.Handlers.ShellItemHandler.UpdateCurrentItem()
at Microsoft.Maui.Controls.Handlers.ShellItemHandler.MapCurrentItem(ShellItemHandler handler, ShellItem item)
at Microsoft.Maui.PropertyMapper`2.<>c__DisplayClass5_0.<Add>b__0(IElementHandler h, IElement v)
at Microsoft.Maui.PropertyMapper.UpdatePropertyCore(String key, IElementHandler viewHandler, IElement virtualView)
at Microsoft.Maui.PropertyMapper.UpdateProperties(IElementHandler viewHandler, IElement virtualView)
at Microsoft.Maui.Handlers.ElementHandler.SetVirtualView(IElement view)
at Microsoft.Maui.Controls.Handlers.ShellItemHandler.SetVirtualView(IElement view)
at Microsoft.Maui.Controls.Platform.ShellView.CreateShellItemView()
at Microsoft.Maui.Controls.Platform.ShellView.SwitchShellItem(ShellItem newItem, Boolean animate)
at Microsoft.Maui.Controls.Handlers.ShellHandler.MapCurrentItem(ShellHandler handler, Shell view)
at Microsoft.Maui.PropertyMapper`2.<>c__DisplayClass5_0.<Add>b__0(IElementHandler h, IElement v)
at Microsoft.Maui.PropertyMapper.UpdatePropertyCore(String key, IElementHandler viewHandler, IElement virtualView)
at Microsoft.Maui.PropertyMapper.UpdateProperty(IElementHandler viewHandler, IElement virtualView, String property)
at Microsoft.Maui.Handlers.ElementHandler.UpdateValue(String property)
at Microsoft.Maui.Controls.Element.OnPropertyChanged(String propertyName)
at Microsoft.Maui.Controls.Shell.OnPropertyChanged(String propertyName)
at Microsoft.Maui.Controls.BindableObject.SetValueActual(BindableProperty property, BindablePropertyContext context, Object value, Boolean currentlyApplying, SetValueFlags attributes, Boolean silent)
at Microsoft.Maui.Controls.BindableObject.SetValueCore(BindableProperty property, Object value, SetValueFlags attributes, SetValuePrivateFlags privateAttributes)
at Microsoft.Maui.Controls.BindableObject.SetValueCore(BindableProperty property, Object value, SetValueFlags attributes)
at Microsoft.Maui.Controls.Element.SetValueFromRenderer(BindableProperty property, Object value)
at Microsoft.Maui.Controls.ShellNavigationManager.GoToAsync(ShellNavigationParameters shellNavigationParameters, ShellNavigationRequest navigationRequest)
at Microsoft.Maui.Controls.Shell.<Initialize>g__SetCurrentItem|172_1()
at Microsoft.Maui.Controls.Shell.<Initialize>b__172_0(Object s, NotifyCollectionChangedEventArgs e)
at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_0(Object state)
at Microsoft.UI.Dispatching.DispatcherQueueSynchronizationContext.<>c__DisplayClass2_0.<Post>b__0()

I could reproduce this in a test by simply doing:

shell.CurrentItem = new ShellContent { Content = page };
shell.Items.Clear();
shell.CurrentItem = new ShellContent { Content = page };

I added appropriate null checking and the problem appears to be solved now.

Fixes: dotnet#13775

You can cause MAUI to throw an NRE by doing:

1. Debug/run, have XAML hot reload enabled

2. Comment out the `ShellContent` in `AppShell.xaml`, hot reload

3. Uncomment the `ShellContent` in `AppShell.xaml`, hot reload

Crashes with:

    System.NullReferenceException: Object reference not set to an instance of an object.
    at Microsoft.Maui.Controls.Handlers.ShellSectionHandler.SetVirtualView(IElement view)
    at Microsoft.Maui.Controls.Handlers.ShellItemHandler.UpdateCurrentItem()
    at Microsoft.Maui.Controls.Handlers.ShellItemHandler.MapCurrentItem(ShellItemHandler handler, ShellItem item)
    at Microsoft.Maui.PropertyMapper`2.<>c__DisplayClass5_0.<Add>b__0(IElementHandler h, IElement v)
    at Microsoft.Maui.PropertyMapper.UpdatePropertyCore(String key, IElementHandler viewHandler, IElement virtualView)
    at Microsoft.Maui.PropertyMapper.UpdateProperties(IElementHandler viewHandler, IElement virtualView)
    at Microsoft.Maui.Handlers.ElementHandler.SetVirtualView(IElement view)
    at Microsoft.Maui.Controls.Handlers.ShellItemHandler.SetVirtualView(IElement view)
    at Microsoft.Maui.Controls.Platform.ShellView.CreateShellItemView()
    at Microsoft.Maui.Controls.Platform.ShellView.SwitchShellItem(ShellItem newItem, Boolean animate)
    at Microsoft.Maui.Controls.Handlers.ShellHandler.MapCurrentItem(ShellHandler handler, Shell view)
    at Microsoft.Maui.PropertyMapper`2.<>c__DisplayClass5_0.<Add>b__0(IElementHandler h, IElement v)
    at Microsoft.Maui.PropertyMapper.UpdatePropertyCore(String key, IElementHandler viewHandler, IElement virtualView)
    at Microsoft.Maui.PropertyMapper.UpdateProperty(IElementHandler viewHandler, IElement virtualView, String property)
    at Microsoft.Maui.Handlers.ElementHandler.UpdateValue(String property)
    at Microsoft.Maui.Controls.Element.OnPropertyChanged(String propertyName)
    at Microsoft.Maui.Controls.Shell.OnPropertyChanged(String propertyName)
    at Microsoft.Maui.Controls.BindableObject.SetValueActual(BindableProperty property, BindablePropertyContext context, Object value, Boolean currentlyApplying, SetValueFlags attributes, Boolean silent)
    at Microsoft.Maui.Controls.BindableObject.SetValueCore(BindableProperty property, Object value, SetValueFlags attributes, SetValuePrivateFlags privateAttributes)
    at Microsoft.Maui.Controls.BindableObject.SetValueCore(BindableProperty property, Object value, SetValueFlags attributes)
    at Microsoft.Maui.Controls.Element.SetValueFromRenderer(BindableProperty property, Object value)
    at Microsoft.Maui.Controls.ShellNavigationManager.GoToAsync(ShellNavigationParameters shellNavigationParameters, ShellNavigationRequest navigationRequest)
    at Microsoft.Maui.Controls.Shell.<Initialize>g__SetCurrentItem|172_1()
    at Microsoft.Maui.Controls.Shell.<Initialize>b__172_0(Object s, NotifyCollectionChangedEventArgs e)
    at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_0(Object state)
    at Microsoft.UI.Dispatching.DispatcherQueueSynchronizationContext.<>c__DisplayClass2_0.<Post>b__0()

I could reproduce this in a test by simply doing:

    shell.CurrentItem = new ShellContent { Content = page };
    shell.Items.Clear();
    shell.CurrentItem = new ShellContent { Content = page };

I added appropriate null checking and the problem appears to be solved
now.
@jonathanpeppers jonathanpeppers added partner/hot-reload-xaml Issues impacting XAML Hot Reload experiences area-controls-shell Shell Navigation, Routes, Tabs, Flyout platform/windows labels May 22, 2023
@jonathanpeppers
jonathanpeppers marked this pull request as ready for review May 22, 2023 19:02
@jonathanpeppers
jonathanpeppers requested a review from PureWeen May 22, 2023 19:08
@jonathanpeppers

jonathanpeppers commented May 22, 2023 •

Copy link
Copy Markdown
Member Author

If this PR is merged before this one:

I can update #15098 to fix the merge conflicts.

((IShellController)_shellSection.FindParentOfType<Shell>()!).RemoveAppearanceObserver(this);

var shell = _shellSection.FindParentOfType<Shell>() as IShellController;
shell?.RemoveAppearanceObserver(this);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we need to store shell locally when the observer is set on line 75 that way we can successfully remove (this) from the list of AppearanceObservers. Otherwise, this Handler will probably remain inside the list of appearance observers inside shell.

List<(IAppearanceObserver Observer, Element Pivot)> _appearanceObservers = new List<(IAppearanceObserver Observer, Element Pivot)>();

Or trigger this tear down code via

class ActionDisposable : IDisposable

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It seems like this is a second problem.

I updated the test to check for this, and then fixed it. But if this fails on iOS/Android it's a problem in other handlers I didn't touch?

@PureWeen
PureWeen enabled auto-merge (squash) May 22, 2023 20:42
@PureWeen
PureWeen merged commit 8565a3e into dotnet:main May 22, 2023
@jonathanpeppers
jonathanpeppers deleted the HotReloadShell branch May 23, 2023 17:43
rmarinho pushed a commit that referenced this pull request May 30, 2023
* [shell] [windows] fix NRE when clearing shell items

Fixes: #13775

You can cause MAUI to throw an NRE by doing:

1. Debug/run, have XAML hot reload enabled

2. Comment out the `ShellContent` in `AppShell.xaml`, hot reload

3. Uncomment the `ShellContent` in `AppShell.xaml`, hot reload

Crashes with:

    System.NullReferenceException: Object reference not set to an instance of an object.
    at Microsoft.Maui.Controls.Handlers.ShellSectionHandler.SetVirtualView(IElement view)
    at Microsoft.Maui.Controls.Handlers.ShellItemHandler.UpdateCurrentItem()
    at Microsoft.Maui.Controls.Handlers.ShellItemHandler.MapCurrentItem(ShellItemHandler handler, ShellItem item)
    at Microsoft.Maui.PropertyMapper`2.<>c__DisplayClass5_0.<Add>b__0(IElementHandler h, IElement v)
    at Microsoft.Maui.PropertyMapper.UpdatePropertyCore(String key, IElementHandler viewHandler, IElement virtualView)
    at Microsoft.Maui.PropertyMapper.UpdateProperties(IElementHandler viewHandler, IElement virtualView)
    at Microsoft.Maui.Handlers.ElementHandler.SetVirtualView(IElement view)
    at Microsoft.Maui.Controls.Handlers.ShellItemHandler.SetVirtualView(IElement view)
    at Microsoft.Maui.Controls.Platform.ShellView.CreateShellItemView()
    at Microsoft.Maui.Controls.Platform.ShellView.SwitchShellItem(ShellItem newItem, Boolean animate)
    at Microsoft.Maui.Controls.Handlers.ShellHandler.MapCurrentItem(ShellHandler handler, Shell view)
    at Microsoft.Maui.PropertyMapper`2.<>c__DisplayClass5_0.<Add>b__0(IElementHandler h, IElement v)
    at Microsoft.Maui.PropertyMapper.UpdatePropertyCore(String key, IElementHandler viewHandler, IElement virtualView)
    at Microsoft.Maui.PropertyMapper.UpdateProperty(IElementHandler viewHandler, IElement virtualView, String property)
    at Microsoft.Maui.Handlers.ElementHandler.UpdateValue(String property)
    at Microsoft.Maui.Controls.Element.OnPropertyChanged(String propertyName)
    at Microsoft.Maui.Controls.Shell.OnPropertyChanged(String propertyName)
    at Microsoft.Maui.Controls.BindableObject.SetValueActual(BindableProperty property, BindablePropertyContext context, Object value, Boolean currentlyApplying, SetValueFlags attributes, Boolean silent)
    at Microsoft.Maui.Controls.BindableObject.SetValueCore(BindableProperty property, Object value, SetValueFlags attributes, SetValuePrivateFlags privateAttributes)
    at Microsoft.Maui.Controls.BindableObject.SetValueCore(BindableProperty property, Object value, SetValueFlags attributes)
    at Microsoft.Maui.Controls.Element.SetValueFromRenderer(BindableProperty property, Object value)
    at Microsoft.Maui.Controls.ShellNavigationManager.GoToAsync(ShellNavigationParameters shellNavigationParameters, ShellNavigationRequest navigationRequest)
    at Microsoft.Maui.Controls.Shell.<Initialize>g__SetCurrentItem|172_1()
    at Microsoft.Maui.Controls.Shell.<Initialize>b__172_0(Object s, NotifyCollectionChangedEventArgs e)
    at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__128_0(Object state)
    at Microsoft.UI.Dispatching.DispatcherQueueSynchronizationContext.<>c__DisplayClass2_0.<Post>b__0()

I could reproduce this in a test by simply doing:

    shell.CurrentItem = new ShellContent { Content = page };
    shell.Items.Clear();
    shell.CurrentItem = new ShellContent { Content = page };

I added appropriate null checking and the problem appears to be solved
now.

* Update test, call RemoveAppearanceObserver more reliably
@github-actions github-actions Bot locked and limited conversation to collaborators Dec 10, 2023
@samhouts samhouts added the fixed-in-8.0.0-preview.5.8529 Look for this fix in 8.0.0-preview.5.8529! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-controls-shell Shell Navigation, Routes, Tabs, Flyout fixed-in-8.0.0-preview.5.8529 Look for this fix in 8.0.0-preview.5.8529! partner/hot-reload-xaml Issues impacting XAML Hot Reload experiences platform/windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Windows] MAUI app crashes when XAML Hot Reloading ShellContent within Shell.xaml

3 participants