Skip to content

Commit

Permalink
Fixes crash when GC runs in the middle of a navigation
Browse files Browse the repository at this point in the history
  • Loading branch information
albyrock87 committed May 8, 2024
1 parent 7e06f56 commit 0dd1d43
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 6 deletions.
11 changes: 11 additions & 0 deletions Samples/Nalu.Maui.Sample/Pages/OnePage.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
xmlns:pageModels="clr-namespace:Nalu.Maui.Sample.PageModels"
xmlns:nalu="https://nalu-development.github.com/nalu/navigation"
xmlns:pages="clr-namespace:Nalu.Maui.Sample.Pages"
x:Class="Nalu.Maui.Sample.Pages.OnePage"
x:DataType="pageModels:OnePageModel"
Title="Page One"
Expand All @@ -26,6 +28,15 @@
<Button Command="{Binding PushThreeCommand}"
Text="Push Three"
HorizontalOptions="Center" />

<Button Command="{nalu:NavigateCommand}"
Text="Push Six Via XAML">
<Button.CommandParameter>
<nalu:RelativeNavigation>
<nalu:NavigationSegment Type="pages:SixPage" />
</nalu:RelativeNavigation>
</Button.CommandParameter>
</Button>
</VerticalStackLayout>
</ScrollView>

Expand Down
10 changes: 10 additions & 0 deletions Samples/Nalu.Maui.Sample/Pages/SixPage.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
xmlns:pageModels="clr-namespace:Nalu.Maui.Sample.PageModels"
xmlns:nalu="https://nalu-development.github.com/nalu/navigation"
xmlns:pages="clr-namespace:Nalu.Maui.Sample.Pages"
x:Class="Nalu.Maui.Sample.Pages.SixPage"
x:DataType="pageModels:SixPageModel"
Title="Page Six"
Expand All @@ -19,6 +21,14 @@
</Label.FormattedText>
</Label>
<Button Text="One" Command="{Binding GoToOneCommand}" />
<Button Command="{nalu:NavigateCommand}"
Text="Push Six Via XAML">
<Button.CommandParameter>
<nalu:RelativeNavigation>
<nalu:NavigationSegment Type="pages:SixPage" />
</nalu:RelativeNavigation>
</Button.CommandParameter>
</Button>
</VerticalStackLayout>
</ContentPage.Content>
</ContentPage>
11 changes: 11 additions & 0 deletions Samples/Nalu.Maui.Sample/Pages/ThreePage.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
xmlns:pageModels="clr-namespace:Nalu.Maui.Sample.PageModels"
xmlns:nalu="https://nalu-development.github.com/nalu/navigation"
xmlns:pages="clr-namespace:Nalu.Maui.Sample.Pages"
x:Class="Nalu.Maui.Sample.Pages.ThreePage"
x:DataType="pageModels:ThreePageModel"
Title="Page Three"
Expand All @@ -21,6 +23,15 @@
Text="Push Four"/>
<Button Command="{Binding ReplaceSixCommand}"
Text="Replace with six"/>

<Button Command="{nalu:NavigateCommand}"
Text="Push Six Via XAML">
<Button.CommandParameter>
<nalu:RelativeNavigation>
<nalu:NavigationSegment Type="pages:SixPage" />
</nalu:RelativeNavigation>
</Button.CommandParameter>
</Button>
</VerticalStackLayout>
</ContentPage.Content>
</ContentPage>
16 changes: 13 additions & 3 deletions Source/Nalu.Maui.Navigation/Internals/FixedRouteFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,26 @@ namespace Nalu;

internal class FixedRouteFactory : RouteFactory
{
private readonly WeakReference<Page> _weakPage;
private Page? _page;

#pragma warning disable IDE0290
public FixedRouteFactory(Page page)
#pragma warning restore IDE0290
{
_weakPage = new WeakReference<Page>(page);
_page = page;
}

public override Element GetOrCreate() => _weakPage.TryGetTarget(out var page) ? page : throw new InvalidOperationException("Page has been collected");
public override Element GetOrCreate()
{
var page = _page;
if (page is not null)
{
_page = null;
return page;
}

throw new InvalidOperationException("Page has already been created.");
}

public override Element GetOrCreate(IServiceProvider services) => GetOrCreate();
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ internal sealed class PageNavigationContext(IServiceScope serviceScope) : IDispo
public IServiceScope ServiceScope => serviceScope;
public bool Entered { get; set; }
public bool Appeared { get; set; }
internal Action? OnDispose { get; set; }

private static readonly BindableProperty _navigationContextProperty = BindableProperty.CreateAttached("PageNavigationContext", typeof(PageNavigationContext), typeof(PageNavigationContext), null);

Expand All @@ -30,5 +31,9 @@ public static void Dispose(Page page)
page.ClearValue(_navigationContextProperty);
}

public void Dispose() => serviceScope.Dispose();
public void Dispose()
{
OnDispose?.Invoke();
serviceScope.Dispose();
}
}
1 change: 1 addition & 0 deletions Source/Nalu.Maui.Navigation/Nalu.Maui.Navigation.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<Description>.NET MAUI developer utilities</Description>
<PackageTags>MAUI</PackageTags>
<RootNamespace>Nalu</RootNamespace>
<NoWarn>NU5129</NoWarn>
</PropertyGroup>

<PropertyGroup Label="Build">
Expand Down
8 changes: 6 additions & 2 deletions Source/Nalu.Maui.Navigation/NavigateCommand.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
namespace Nalu;

using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Windows.Input;
using Nalu.Internals;
Expand Down Expand Up @@ -61,9 +62,12 @@ public async void Execute(object? parameter)
{
await navigationService.GoToAsync(navigation).ConfigureAwait(true);
}
catch
catch (Exception ex)
{
// Ignore
if (Debugger.IsAttached)
{
Console.WriteLine(ex);
}
}

Interlocked.Exchange(ref _canExecute, 1);
Expand Down
12 changes: 12 additions & 0 deletions Source/Nalu.Maui.Navigation/ShellInfo/ShellProxy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,21 @@ public async Task PushAsync(string segmentName, Page page)

var baseRoute = _navigationTarget ?? _shell.CurrentState.Location.OriginalString;
var finalRoute = $"{baseRoute}/{segmentName}";

Routing.UnRegisterRoute(finalRoute);
Routing.RegisterRoute(finalRoute, new FixedRouteFactory(page));

// Apparently shell does not work well when the same segment is being used differently
// inside the same ShellContent:
// - //MyShellContent/MyPage
// - //MyShellContent/OtherPage/MyPage
// - //MyShellContent/MyPage/MyPage
// It always falls under the first one, even when we clearly specify an absolute Uri
// For now there's no apparent way to fix the recursive route, while on the other use cases
// We can just unregister the route when the page is disposed
var pageNavigationContext = PageNavigationContext.Get(page);
pageNavigationContext.OnDispose = () => Routing.UnRegisterRoute(finalRoute);

if (_navigationTarget != null)
{
_navigationTarget = finalRoute;
Expand Down

0 comments on commit 0dd1d43

Please sign in to comment.