Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Controls/src/Core/Window/Window.cs
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,12 @@ void IWindow.Destroying()

AlertManager.Unsubscribe();
Application?.RemoveWindow(this);

var mauiContext = Handler?.MauiContext as MauiContext;
Handler?.DisconnectHandler();

// Dispose the window-scoped service scope
mauiContext?.DisposeWindowScope();
}

void IWindow.Resumed()
Expand Down
94 changes: 88 additions & 6 deletions src/Controls/tests/Core.UnitTests/WindowsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Reflection;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Maui.Graphics;
using Xunit;

Expand Down Expand Up @@ -856,13 +857,94 @@ public void BindingIsActivatedProperty()

Assert.False(vm.IsWindowActive);
}
}

class ViewModel
{
public bool IsWindowActive
[Fact]
public void WindowServiceScopeIsDisposedOnDestroying()
{
var serviceCollection = new ServiceCollection();
serviceCollection.AddTransient<TestScopedService>();
var serviceProvider = serviceCollection.BuildServiceProvider();

var scope = serviceProvider.CreateScope();
var mauiContext = new MauiContext(scope.ServiceProvider);
mauiContext.SetWindowScope(scope);

var window = new TestWindow(new ContentPage());
var handler = new WindowHandlerStub();
handler.SetMauiContext(mauiContext);
window.Handler = handler;

// Verify the scope service works before disposal
var service = mauiContext.Services.GetService<TestScopedService>();
Assert.NotNull(service);

// Destroy the window - this should dispose the scope
((IWindow)window).Destroying();

// After disposal, the scope should be disposed
// We can't directly test if scope is disposed, but we can test that trying to use it throws
Assert.Throws<ObjectDisposedException>(() => scope.ServiceProvider.GetService<TestScopedService>());
}

class ViewModel
{
public bool IsWindowActive
{
get;
set;
}
}

[Fact]
public void WindowServiceScopeHandlesNullScope()
{
// Test that destroying a window without a scope doesn't throw
var mauiContext = new MockMauiContext();
var window = new TestWindow(new ContentPage());
var handler = new WindowHandlerStub();
handler.SetMauiContext(mauiContext);
window.Handler = handler;

// This should not throw even though there's no scope to dispose
((IWindow)window).Destroying();
}

[Fact]
public void WindowServiceScopeWorksWithWindowCreationFlow()
{
// Test the full flow as it would happen in real usage
var serviceCollection = new ServiceCollection();
serviceCollection.AddScoped<TestScopedService>();
var rootServiceProvider = serviceCollection.BuildServiceProvider();

var appContext = new MauiContext(rootServiceProvider);

// Simulate the window creation flow
var windowContext = appContext.MakeWindowScope(new object(), out var scope);

// Verify we can get scoped services
var service1 = windowContext.Services.GetRequiredService<TestScopedService>();
var service2 = windowContext.Services.GetRequiredService<TestScopedService>();

// Should be the same instance since it's scoped
Assert.Same(service1, service2);

// Create window and set up handler
var window = new TestWindow(new ContentPage());
var handler = new WindowHandlerStub();
handler.SetMauiContext(windowContext);
window.Handler = handler;

// Destroy the window
((IWindow)window).Destroying();

// Scope should be disposed
Assert.Throws<ObjectDisposedException>(() => scope.ServiceProvider.GetService<TestScopedService>());
}

private class TestScopedService
{
get; set;
public string TestProperty { get; set; } = "test";
}
}
}
}
12 changes: 12 additions & 0 deletions src/Core/src/MauiContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public class MauiContext : IMauiContext
{
readonly WrappedServiceProvider _services;
readonly Lazy<IMauiHandlersFactory> _handlers;
IServiceScope? _windowScope;
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a WeakReference ?

Copy link
Member

Choose a reason for hiding this comment

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

@rmarinho no, the service scope's life is already tied to the window, it's not something that's just going to naturally finalize and dispose.

Having this here doesn't cause a memory leak

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using a WeakReference here wouldn't be appropriate because we need to maintain a strong reference to the scope to ensure proper disposal. If we used a WeakReference, the scope could be garbage collected before we dispose it, potentially causing memory leaks. The scope needs to stay alive until we explicitly dispose it when the window is destroyed.


#if ANDROID
readonly Lazy<Context?> _context;
Expand Down Expand Up @@ -53,6 +54,17 @@ internal void AddWeakSpecific<TService>(TService instance)
_services.AddSpecific(typeof(TService), static state => ((WeakReference)state).Target, new WeakReference(instance));
}

internal void SetWindowScope(IServiceScope scope)
{
_windowScope = scope;
}

internal void DisposeWindowScope()
{
_windowScope?.Dispose();
Copy link
Member

Choose a reason for hiding this comment

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

@copilot set this to null after using this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! Set _windowScope = null after disposal to clear the reference. Commit: c0f7dc0

_windowScope = null;
}

class WrappedServiceProvider : IServiceProvider
{
readonly ConcurrentDictionary<Type, (object, Func<object, object?>)> _scopeStatic = new();
Expand Down
4 changes: 3 additions & 1 deletion src/Core/src/MauiContextExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ public static IMauiContext MakeApplicationScope(this IMauiContext mauiContext, N
public static IMauiContext MakeWindowScope(this IMauiContext mauiContext, NativeWindow platformWindow, out IServiceScope scope)
{
// Create the window-level scopes that will only be used for the lifetime of the window
// TODO: We need to dispose of these services once the window closes
scope = mauiContext.Services.CreateScope();

#if ANDROID
Expand All @@ -56,6 +55,9 @@ public static IMauiContext MakeWindowScope(this IMauiContext mauiContext, Native
var scopedContext = new MauiContext(scope.ServiceProvider);
#endif

// Store the scope in the scoped context so it can be disposed when the window is destroyed
scopedContext.SetWindowScope(scope);

scopedContext.AddWeakSpecific(platformWindow);

#if ANDROID
Expand Down
Loading