Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception from ~DisposableCallback when component is IAsyncDisposable #425

Closed
codedevote opened this issue May 22, 2023 · 1 comment
Closed

Comments

@codedevote
Copy link
Contributor

Hi,

I recently commented on the discussion in #414 , but there were no more insights provided that helped me. So I started to deeply investigate my issue. I set up my dev env to be able to debug your library's code. First thing I changed (I can send a PR for that if you want) was to include more information in the exception that is thrown from the finalizer in DisposableCallback by adding type of subject in StateSubscriber. It always throws with the same source (id), so I added subject.GetType().FullName to the id, which immediately helped me understand which component is the offending one.

Going forward I understood, that your implementation of Dispose() in FluxorComponent is following best practice implementation of IDispoable pattern . BUT: If you combine that with what MS suggests as best practice for IAsyncDisposable implementation (https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-disposeasync), we end up with the issue I encountered. They suggest to call Dispose(false) in case of async dispose, which skips your cleanup in Dispose(true) and later throwas an exception in ~DisposableCallback.

I was able to fix my issue by switching to FluxorComponent.Dispose(true) call in my DisposeAsync(), and since it catches exceptions anyways (being a blazor server app and required to deal with CircuitDisconnect exceptions on disposal), I am fine with that solution.
Nonetheless, I thought I share my insights with you, maybe you have some thoughts on that as well.

@mrpmorris
Copy link
Owner

Thank you!

A PR would be very welcome :)

codedevote added a commit to codedevote/Fluxor that referenced this issue May 25, 2023
@mrpmorris mrpmorris mentioned this issue Jun 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants