From 2e5813cddb42ffe3aac34eb7e43606eb14b6a20e Mon Sep 17 00:00:00 2001 From: Safia Abdalla Date: Tue, 30 Jan 2024 16:37:21 -0800 Subject: [PATCH 1/6] Dispose of ChangeToken --- .../src/Builder/RazorComponentEndpointDataSource.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs b/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs index 58a7ac8d656f..c29461ebfaf5 100644 --- a/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs +++ b/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs @@ -28,6 +28,7 @@ internal class RazorComponentEndpointDataSource<[DynamicallyAccessedMembers(Comp private List? _endpoints; private CancellationTokenSource _cancellationTokenSource; private IChangeToken _changeToken; + private IDisposable? _disposable; // Internal for testing. internal ComponentApplicationBuilder Builder => _builder; @@ -141,7 +142,7 @@ private void UpdateEndpoints() oldCancellationTokenSource?.Cancel(); if (_hotReloadService is { MetadataUpdateSupported : true }) { - ChangeToken.OnChange(_hotReloadService.GetChangeToken, UpdateEndpoints); + _disposable = ChangeToken.OnChange(_hotReloadService.GetChangeToken, UpdateEndpoints); } } } @@ -153,4 +154,10 @@ public override IChangeToken GetChangeToken() Debug.Assert(_endpoints != null); return _changeToken; } + + public void Dispose() + { + _disposable?.Dispose(); + _disposable = null; + } } From 361b30879b09cb509a276a1099a09926a421ddfc Mon Sep 17 00:00:00 2001 From: Jackson Davis Date: Tue, 30 Jan 2024 21:45:28 -0800 Subject: [PATCH 2/6] Attempted fix of razor hotreload leak. This disposes the old change tokens before overwriting. Note that I don't believe the dispose method that was added previously is actually ever called --- .../src/Builder/RazorComponentEndpointDataSource.cs | 6 ++++++ .../Endpoints/src/DependencyInjection/HotReloadService.cs | 1 + 2 files changed, 7 insertions(+) diff --git a/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs b/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs index c29461ebfaf5..01fe4dbaa2c3 100644 --- a/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs +++ b/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs @@ -140,8 +140,13 @@ private void UpdateEndpoints() _cancellationTokenSource = new CancellationTokenSource(); _changeToken = new CancellationChangeToken(_cancellationTokenSource.Token); oldCancellationTokenSource?.Cancel(); + oldCancellationTokenSource?.Dispose(); if (_hotReloadService is { MetadataUpdateSupported : true }) { + if (_disposable != null) + { + _disposable?.Dispose(); + } _disposable = ChangeToken.OnChange(_hotReloadService.GetChangeToken, UpdateEndpoints); } } @@ -155,6 +160,7 @@ public override IChangeToken GetChangeToken() return _changeToken; } + // NOTE: Safia, I don't think this will ever get called. public void Dispose() { _disposable?.Dispose(); diff --git a/src/Components/Endpoints/src/DependencyInjection/HotReloadService.cs b/src/Components/Endpoints/src/DependencyInjection/HotReloadService.cs index 51680ca61034..a0a37d7a1691 100644 --- a/src/Components/Endpoints/src/DependencyInjection/HotReloadService.cs +++ b/src/Components/Endpoints/src/DependencyInjection/HotReloadService.cs @@ -32,6 +32,7 @@ private void NotifyUpdateApplication(Type[]? changedTypes) { var current = Interlocked.Exchange(ref _tokenSource, new CancellationTokenSource()); current.Cancel(); + current.Dispose(); } public void Dispose() From 6e30d430940f096156221d5388c59f216f5d9036 Mon Sep 17 00:00:00 2001 From: Jackson Davis Date: Wed, 31 Jan 2024 10:38:48 -0800 Subject: [PATCH 3/6] Remove unused disposable method from endpoint data source --- .../src/Builder/RazorComponentEndpointDataSource.cs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs b/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs index 01fe4dbaa2c3..551dec086648 100644 --- a/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs +++ b/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs @@ -159,11 +159,4 @@ public override IChangeToken GetChangeToken() Debug.Assert(_endpoints != null); return _changeToken; } - - // NOTE: Safia, I don't think this will ever get called. - public void Dispose() - { - _disposable?.Dispose(); - _disposable = null; - } } From 4deaf27d388b899d4032af884e0b0574d21a8f2d Mon Sep 17 00:00:00 2001 From: Jackson Davis Date: Wed, 31 Jan 2024 15:59:55 -0800 Subject: [PATCH 4/6] Switch to using the ClearCache event from the hot reload service for disposing the change token. 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. --- .../src/Builder/RazorComponentEndpointDataSource.cs | 10 ++++++++++ .../src/DependencyInjection/HotReloadService.cs | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs b/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs index 551dec086648..b95eb86561f5 100644 --- a/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs +++ b/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs @@ -46,6 +46,7 @@ public RazorComponentEndpointDataSource( _renderModeEndpointProviders = renderModeEndpointProviders.ToArray(); _factory = factory; _hotReloadService = hotReloadService; + HotReloadService.ClearCacheEvent += OnHotReloadClearCache; DefaultBuilder = new RazorComponentsEndpointConventionBuilder( _lock, builder, @@ -151,6 +152,15 @@ private void UpdateEndpoints() } } } + + public void OnHotReloadClearCache(Type[]? types) + { + if (_disposable != null) + { + _disposable?.Dispose(); + _disposable = null; + } + } public override IChangeToken GetChangeToken() { diff --git a/src/Components/Endpoints/src/DependencyInjection/HotReloadService.cs b/src/Components/Endpoints/src/DependencyInjection/HotReloadService.cs index a0a37d7a1691..ad3c985b564f 100644 --- a/src/Components/Endpoints/src/DependencyInjection/HotReloadService.cs +++ b/src/Components/Endpoints/src/DependencyInjection/HotReloadService.cs @@ -18,6 +18,7 @@ public HotReloadService() private CancellationTokenSource _tokenSource = new(); private static event Action? UpdateApplicationEvent; + internal static event Action? ClearCacheEvent; public bool MetadataUpdateSupported { get; internal set; } @@ -27,6 +28,11 @@ public static void UpdateApplication(Type[]? changedTypes) { UpdateApplicationEvent?.Invoke(changedTypes); } + + public static void ClearCache(Type[]? types) + { + ClearCacheEvent?.Invoke(types); + } private void NotifyUpdateApplication(Type[]? changedTypes) { From 18a67ace0bf1144f8d1c6a888335d1a65503b31c Mon Sep 17 00:00:00 2001 From: Jackson Davis Date: Wed, 31 Jan 2024 16:29:25 -0800 Subject: [PATCH 5/6] Make the disposable change token thread-safe --- .../src/Builder/RazorComponentEndpointDataSource.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs b/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs index b95eb86561f5..1d9767f9a39d 100644 --- a/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs +++ b/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs @@ -28,7 +28,7 @@ internal class RazorComponentEndpointDataSource<[DynamicallyAccessedMembers(Comp private List? _endpoints; private CancellationTokenSource _cancellationTokenSource; private IChangeToken _changeToken; - private IDisposable? _disposable; + private IDisposable? _disposable; // THREADING: protected by _lock // Internal for testing. internal ComponentApplicationBuilder Builder => _builder; @@ -155,10 +155,13 @@ private void UpdateEndpoints() public void OnHotReloadClearCache(Type[]? types) { - if (_disposable != null) + lock (_lock) { - _disposable?.Dispose(); - _disposable = null; + if (_disposable != null) + { + _disposable?.Dispose(); + _disposable = null; + } } } From 974f85e9a2187281f3ef7b2b8a06d21e6c9f504f Mon Sep 17 00:00:00 2001 From: Jackson Davis Date: Thu, 1 Feb 2024 12:04:25 -0800 Subject: [PATCH 6/6] PR feedback --- .../src/Builder/RazorComponentEndpointDataSource.cs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs b/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs index 1d9767f9a39d..992bd4fe498d 100644 --- a/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs +++ b/src/Components/Endpoints/src/Builder/RazorComponentEndpointDataSource.cs @@ -144,10 +144,7 @@ private void UpdateEndpoints() oldCancellationTokenSource?.Dispose(); if (_hotReloadService is { MetadataUpdateSupported : true }) { - if (_disposable != null) - { - _disposable?.Dispose(); - } + _disposable?.Dispose(); _disposable = ChangeToken.OnChange(_hotReloadService.GetChangeToken, UpdateEndpoints); } } @@ -157,11 +154,8 @@ public void OnHotReloadClearCache(Type[]? types) { lock (_lock) { - if (_disposable != null) - { - _disposable?.Dispose(); - _disposable = null; - } + _disposable?.Dispose(); + _disposable = null; } }