mrpmorris / Fluxor

Fluxor is a zero boilerplate Flux/Redux library for Microsoft .NET and Blazor.
MIT License
1.22k stars 139 forks source link

Fix: deadlock caused by event invocation within lock #469

Open huntercfreeman opened 5 months ago

huntercfreeman commented 5 months ago

I am running Blazor natively with photino.Blazor (github).

I experience a deadlock in Feature.cs due to a lock which goes on to cause another lock on the same object.

Presumably the intention is that the same thread is doing this double lock, but I encounter two separate threads trying to enter the lock.

It starts with me dispatching an Action, in my case I'm clicking the button in the following image.

100_nothing

That button click results in this function being ran Feature.ReceiveDispatchNotificationFromStore

200_ReceiveDispatchNotificationFromStore

This function, at the end, invokes the setter for the property named "State".

300_State 400_Setter

The setter for "State" is here. When I enter the lock, my thread according to the Visual Studio debugger is identified by "40512 .NET Thread"

500_state_setter_thread

After the 'State' has its state changed, an event is invoked via the 'TriggerStateChangedCallbacksThrottler'.

600_state_setter_event

The code then goes to the StateChanged event's 'add'

700_second_lock

A lock on 'SyncRoot' is performed again. However, if I check my thread, the Visual Studio debuggers says I'm on the thread identified by "40600 Main Thread". This is different from the previous time the lock was entered, of which has not yet been left.

700_second_lock_thread

Therefore, I encounter a deadlock and the app freezes.

Within the setter for State, If I declare the "bool stateHasChanged" outside of the lock.

I can allow the lock to close, prior to invoking the event. This "seems" to have fixed my issue. I might have been doing something wrong from the get-go to even end up in this scenario however, I'm unsure.

I do not believe this can cause any issues, because I believe this event only notifies Blazor components to re-render?

800_the_change 900_try_change 1000_result

Additional information: The code I am encountering this issue with is public and can be found here if that is of use: Luthetus/Luthetus.Ide (github)

Furthermore, the source code for the Blazor component in the images is at this link (line number for the method invoked is included in the link): https://github.com/Luthetus/Luthetus.Ide/blob/main/Luthetus.Ide/Source/Lib/Luthetus.Ide.RazorLib/DotNetSolutions/Displays/DotNetSolutionFormDisplay.razor.cs#L60

huntercfreeman commented 5 months ago

A similar deadlock was found in Dispatcher.cs. The second commit is how I fixed this one.

mrpmorris commented 4 months ago

Would you be available to discuss this on a video call?

I'm available Monday-Friday from 5PM UK time.

huntercfreeman commented 4 months ago

Yes, is Monday (2024-02-26) good? I checked and 5PM UK is 12PM EST so the time works.

If the day is not good, I'm available Monday-Friday 5PM UK any day you'd like.

mrpmorris commented 4 months ago

Wednesday would be best for me this week.

huntercfreeman commented 4 months ago

Wednesday works for me too

mrpmorris commented 4 months ago

Wednesday works for me too

I've just sent you a meeting link.