[Menu] More fixes for dispose error #4258
Merged
+18
−3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dvoituron
approved these changes
Oct 21, 2025
|
@vnbaaij for me it is still not working. I still get a disposed error: This can be reproduced with the steps from this: #4248 (comment) |
|
I'm gonna check it out this evening, might join this pr for some pair programming :) |
|
@vnbaaij I set up a try catch here to debug this and the root of this issue is that the dotNetReference has been disposed but is not null. |
|
✅ All tests passed successfully Details on your Workflow / Core Tests page. |
|
@vnbaaij I found a working solution. In line 28 add: private bool _disposed;In line 244 change to: if (_jsModule is not null && _anchoredRegionModule is not null && _dotNetHelper is not null && !_disposed)as well in line 251 to: if (_jsModule is not null && _anchoredRegionModule is not null && _dotNetHelper is not null && !_disposed && _reinitializeEventListeners)And then update the dispose method to: public async ValueTask DisposeAsync()
{
if (_disposed)
{
return;
}
_disposed = true;
_dotNetHelper?.Dispose();
try
{
if (_jsModule is not null)
{
await _jsModule.InvokeVoidAsync("dispose", Anchor);
await _jsModule.DisposeAsync();
_jsModule = null;
}
if (_anchoredRegionModule is not null)
{
await _anchoredRegionModule.DisposeAsync();
_anchoredRegionModule = null;
}
}
catch (Exception ex) when (ex is JSDisconnectedException ||
ex is OperationCanceledException)
{
// The JSRuntime side may routinely be gone already if the reason we're disposing is that
// the client disconnected. This is not an error.
}
finally
{
GC.SuppressFinalize(this);
}
} |
Summary - Unit Tests Code CoverageSummary
CoverageMicrosoft.FluentUI.AspNetCore.Components - 61.2%
|
|
At least my provided test code doesn't seem to crash when we are doing it this way. |
|
Yeah, I think this solution is in line with what @Tyme-Bleyaert meant. |
|
@vnbaaij we should probably move the disposed check within We should probably do if( _disposed)
{
return;
}right in the beginning and remove it from the 2 if statements |
|
nevermind. I just tried it when it's on top. This gets the error right back to me. |
|
I change the |
- Undo nullable for js object references
MarvinKlein1508
suggested changes
Oct 22, 2025
|
Sorry wrong button. Should we also move _dotNetHelper = DotNetObjectReference.Create(this);into the constructor? |
No, we always do that in |
|
@vnbaaij you are right. I was thinking wrong here. It wouldn't make a difference anyways since the method is only run on firstRender. |
|
@vnbaaij I tried my demo now in WebAssembly and it seems like the disposing here is a bit faster which triggers the exception earlier. |
MarvinKlein1508
approved these changes
Oct 22, 2025
Tyme-Bleyaert
approved these changes
Oct 22, 2025
dvoituron
approved these changes
Oct 23, 2025
|
I have upgraded my project to the latest v4 preview release and I can confirm that this seems to be working now! :) No more crashes for me :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
More fixes for #4248:
IJSObjectReferences nullableDisposeAsyncDotNetObjectReferenceinOnAfterRenderAsyncI'll wait with merging this time till we are all on the same page :)