Eliminate deadlock - Fixes #426

[mrpmorris]

Use ConcurrentQueue instead of lock() for queued actions

[mfmadsen]

@mrpmorris, I'm not sure that the changes I suggested in #427 are necessary and relevant if the changes you propose here in this PR are merged. With the changes you propose, the deadlock fixed in #427 can no longer occur as the passing test also shows. However, the change you propose does change the behavior of the library: Before the change, a consumer calling Dispatcher.Dispatch after the store was initialized, could rely on all reducers having run to completion after the call to .Dispatch completed. With the proposed changes, this is no longer the case. Now when .Dispatch completes, the consumer can only know for sure that the dispatched action will be reduced at some point and before any actions dispatched afterwards.

I'm not sure if this change is a major change or not. Consumers should probably not rely on .Dispatch having a side effects immediately, but should only rely on side effects occurring eventually and before side effects from actions dispatched subsequently (as is the case with the changes you propose in this PR). But I'm not sure if there are any protocol or guarantees that breaks due to this change...

Another thing: I believe there is a potential race condition in regards to the check on IsDispatchingin line 169 in Store.cs where an action theoretically could have been enqueued in line 158 while another thread just completed processing the queue (at line 288/289) but have not yet set IsDispatching to false. In that case the action just enqueued would not be dequeued immediately but first on any subsequent dispatching. This can be fixed by adding if (QueuedActions.Count > 0) { this.DequeueActions(); } at line 306 (just after the loop calling TriggerEffects. That way it is ensured the queue is always emptied regardless of race conditions.

[mrpmorris]

I think this will need more thought :)

I'm trying to remember why I didn't want to trigger effects where you did. Perhaps I was being a bit of a perfectionist and ensuring there were no side effects whilst running the reducers.

[mfmadsen]

I think it makes sense to not change when effects are triggered (ie., keep it like you implemented it initially) and instead make this change you propose in this PR regarding removing the lock in Dispatcher.

In my opinion consumers should not rely on a specific state having been reached immediately after having called Dispatch. What’s important is that effects can rely on reducers having been run before the effect. But then again, I might overlook something :-)

However, the race condition on IsDispatching is probably more important to fix with this PR as calls to .Dispatch() no longer wait on each other.

[mrpmorris]

Merged into 5.9 and released.

Thank you very much for your contribution + time spent discussing the issue!