Skip to content

Commit 414f0d4

Browse files
[release/8.0] [Blazor] Fix hot reload memory leak (#53928)
* Resolve change token leak in Blazor hot reload (#53750) Fix of razor hotreload change token leak. This disposes the old change tokens after the ClearCache event or before overwriting. If something goes wrong and this isn't cleared before the next invocation of UpdateEndpoints on the razor data source, clear it and dispose of it then. * Add unit test to confirm change token is disposed during (#53827) * Add unit test to confirm change token is disposed during razer hot reload. * Per Makinnon's feedback, switch to a callback model to create the wrapped disposable for this unit test. * Update src/Components/Endpoints/test/HotReloadServiceTests.cs --------- Co-authored-by: Mackinnon Buck <[email protected]> --------- Co-authored-by: jacdavis <[email protected]>
1 parent 6c42486 commit 414f0d4

File tree

3 files changed

+69
-1
lines changed

3 files changed

+69
-1
lines changed

src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ internal class RazorComponentEndpointDataSource<[DynamicallyAccessedMembers(Comp
2828
private List<Endpoint>? _endpoints;
2929
private CancellationTokenSource _cancellationTokenSource;
3030
private IChangeToken _changeToken;
31+
private IDisposable? _disposableChangeToken; // THREADING: protected by _lock
32+
33+
public Func<IDisposable, IDisposable> SetDisposableChangeTokenAction = disposableChangeToken => disposableChangeToken;
3134

3235
// Internal for testing.
3336
internal ComponentApplicationBuilder Builder => _builder;
@@ -45,6 +48,7 @@ public RazorComponentEndpointDataSource(
4548
_renderModeEndpointProviders = renderModeEndpointProviders.ToArray();
4649
_factory = factory;
4750
_hotReloadService = hotReloadService;
51+
HotReloadService.ClearCacheEvent += OnHotReloadClearCache;
4852
DefaultBuilder = new RazorComponentsEndpointConventionBuilder(
4953
_lock,
5054
builder,
@@ -139,12 +143,23 @@ private void UpdateEndpoints()
139143
_cancellationTokenSource = new CancellationTokenSource();
140144
_changeToken = new CancellationChangeToken(_cancellationTokenSource.Token);
141145
oldCancellationTokenSource?.Cancel();
146+
oldCancellationTokenSource?.Dispose();
142147
if (_hotReloadService is { MetadataUpdateSupported : true })
143148
{
144-
ChangeToken.OnChange(_hotReloadService.GetChangeToken, UpdateEndpoints);
149+
_disposableChangeToken?.Dispose();
150+
_disposableChangeToken = SetDisposableChangeTokenAction(ChangeToken.OnChange(_hotReloadService.GetChangeToken, UpdateEndpoints));
145151
}
146152
}
147153
}
154+
155+
public void OnHotReloadClearCache(Type[]? types)
156+
{
157+
lock (_lock)
158+
{
159+
_disposableChangeToken?.Dispose();
160+
_disposableChangeToken = null;
161+
}
162+
}
148163

149164
public override IChangeToken GetChangeToken()
150165
{

src/Components/Endpoints/src/DependencyInjection/HotReloadService.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ public HotReloadService()
1818

1919
private CancellationTokenSource _tokenSource = new();
2020
private static event Action<Type[]?>? UpdateApplicationEvent;
21+
internal static event Action<Type[]?>? ClearCacheEvent;
2122

2223
public bool MetadataUpdateSupported { get; internal set; }
2324

@@ -27,11 +28,17 @@ public static void UpdateApplication(Type[]? changedTypes)
2728
{
2829
UpdateApplicationEvent?.Invoke(changedTypes);
2930
}
31+
32+
public static void ClearCache(Type[]? types)
33+
{
34+
ClearCacheEvent?.Invoke(types);
35+
}
3036

3137
private void NotifyUpdateApplication(Type[]? changedTypes)
3238
{
3339
var current = Interlocked.Exchange(ref _tokenSource, new CancellationTokenSource());
3440
current.Cancel();
41+
current.Dispose();
3542
}
3643

3744
public void Dispose()

src/Components/Endpoints/test/HotReloadServiceTests.cs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,52 @@ public void NotifiesCompositeEndpointDataSource()
137137
Assert.Empty(compositeEndpointDataSource.Endpoints);
138138
}
139139

140+
private sealed class WrappedChangeTokenDisposable : IDisposable
141+
{
142+
public bool IsDisposed { get; private set; }
143+
private readonly IDisposable _innerDisposable;
144+
145+
public WrappedChangeTokenDisposable(IDisposable innerDisposable)
146+
{
147+
_innerDisposable = innerDisposable;
148+
}
149+
150+
public void Dispose()
151+
{
152+
IsDisposed = true;
153+
_innerDisposable.Dispose();
154+
}
155+
}
156+
157+
[Fact]
158+
public void ConfirmChangeTokenDisposedHotReload()
159+
{
160+
// Arrange
161+
var builder = CreateBuilder(typeof(ServerComponent));
162+
var services = CreateServices(typeof(MockEndpointProvider));
163+
var endpointDataSource = CreateDataSource<App>(builder, services);
164+
165+
WrappedChangeTokenDisposable wrappedChangeTokenDisposable = null;
166+
167+
endpointDataSource.SetDisposableChangeTokenAction = (IDisposable disposableChangeToken) => {
168+
wrappedChangeTokenDisposable = new WrappedChangeTokenDisposable(disposableChangeToken);
169+
return wrappedChangeTokenDisposable;
170+
};
171+
172+
var endpoint = Assert.IsType<RouteEndpoint>(Assert.Single(endpointDataSource.Endpoints));
173+
Assert.Equal("/server", endpoint.RoutePattern.RawText);
174+
Assert.DoesNotContain(endpoint.Metadata, (element) => element is TestMetadata);
175+
176+
// Make a modification and then perform a hot reload.
177+
endpointDataSource.Conventions.Add(builder =>
178+
builder.Metadata.Add(new TestMetadata()));
179+
HotReloadService.UpdateApplication(null);
180+
HotReloadService.ClearCache(null);
181+
182+
// Confirm the change token is disposed after ClearCache
183+
Assert.True(wrappedChangeTokenDisposable.IsDisposed);
184+
}
185+
140186
private class TestMetadata { }
141187

142188
private ComponentApplicationBuilder CreateBuilder(params Type[] types)

0 commit comments

Comments
 (0)