Merge branch 'master' of https://github.com/mrpmorris/Fluxor

Docs/releases.md

@@ -1,5 +1,8 @@
 # Releases
 
+## New in 5.8
+* Fixes potential for deadlock ([#407](https://github.com/mrpmorris/Fluxor/issues/407))
+
 ## New in 5.7
 * Fixes memory leak when using `ActionSubscriber` or `SubscribeToAction` ([#378](https://github.com/mrpmorris/Fluxor/issues/378))
 

Source/Lib/Fluxor/Store.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
@@ -20,7 +21,7 @@ public class Store : IStore, IActionSubscriber, IDisposable
 		private readonly List<IEffect> Effects = new();
 		private readonly List<IMiddleware> Middlewares = new();
 		private readonly List<IMiddleware> ReversedMiddlewares = new();
-		private readonly Queue<object> QueuedActions = new();
+		private readonly ConcurrentQueue<object> QueuedActions = new();
 		private readonly TaskCompletionSource<bool> InitializedCompletionSource = new();
 		private readonly ActionSubscriber ActionSubscriber;
 
@@ -144,32 +145,34 @@ void IDisposable.Dispose()
 
 		private void ActionDispatched(object sender, ActionDispatchedEventArgs e)
 		{
-			lock (SyncRoot)
-			{
-				// Do not allow task dispatching inside a middleware-change.
-				// These change cycles are for things like "jump to state" in Redux Dev Tools
-				// and should be short lived.
-				// We avoid dispatching inside a middleware change because we don't want UI events (like component Init)
-				// that trigger actions (such as fetching data from a server) to execute
-				if (IsInsideMiddlewareChange)
-					return;
-
-				// If a dequeue is already in progress, we will just
-				// let this new action be added to the queue and then exit
-				// Note: This is to cater for the following scenario
-				//	1: An action is dispatched
-				//	2: An effect is triggered
-				//	3: The effect immediately dispatches a new action
-				// The Queue ensures it is processed after its triggering action has completed rather than immediately
-				QueuedActions.Enqueue(e.Action);
-
-				// HasActivatedStore is set to true when the page finishes loading
-				// At which point DequeueActions will be called
-				if (!HasActivatedStore)
-					return;
+			// Do not allow task dispatching inside a middleware-change.
+			// These change cycles are for things like "jump to state" in Redux Dev Tools
+			// and should be short lived.
+			// We avoid dispatching inside a middleware change because we don't want UI events (like component Init)
+			// that trigger actions (such as fetching data from a server) to execute
+			if (IsInsideMiddlewareChange)
+				return;
 
-				DequeueActions();
-			};
+
+			// This is a concurrent queue, so is safe even if dequeuing is already in progress
+			QueuedActions.Enqueue(e.Action);
+
+			// HasActivatedStore is set to true when the page finishes loading
+			// At which point DequeueActions will be called
+			// So if it hasn't been activated yet, just exit and wait for that to happen
+			if (!HasActivatedStore)
+				return;
+
+			// If a dequeue is still going then it will deal with the event we just
+			// queued, so we can exit at this point.
+			// This prevents a re-entrant deadlock
+			if (!IsDispatching)
+			{
+				lock (SyncRoot)
+				{
+					DequeueActions();
+				};
+			}
 		}
 
 		private void EndMiddlewareChange(IDisposable[] disposables)
@@ -277,10 +280,8 @@ private void DequeueActions()
 			IsDispatching = true;
 			try
 			{
-				while (QueuedActions.Count > 0)
+				while (QueuedActions.TryDequeue(out object nextActionToProcess))
 				{
-					object nextActionToProcess = QueuedActions.Dequeue();
-
 					// Only process the action if no middleware vetos it
 					if (Middlewares.All(x => x.MayDispatchAction(nextActionToProcess)))
 					{
@@ -300,7 +301,7 @@ private void DequeueActions()
 			{
 				IsDispatching = false;
 			}
-			foreach(var dispatchedAction in dispatchedActions)
+			foreach (var dispatchedAction in dispatchedActions)
 				TriggerEffects(dispatchedAction);
 		}
 	}

Source/Tests/Fluxor.UnitTests/StoreTests/ThreadingTests/DispatchReentrancyTests/DispatchReentrancyTests.cs

@@ -0,0 +1,53 @@
+using Fluxor.UnitTests.StoreTests.ThreadingTests.DispatchReentrancyTests.SupportFiles;
+using System;
+using System.Threading;
+using System.Threading.Tasks;
+using Xunit;
+
+namespace Fluxor.UnitTests.StoreTests.ThreadingTests.DispatchReentrancyTests
+{
+	public class DispatchReentrancyTests
+	{
+		private readonly IDispatcher Dispatcher;
+		private readonly IStore Subject;
+		private readonly IFeature<CounterState> Feature;
+
+		[Fact]
+		public async Task WhenObserverSubscribesToAnAction_AndDispatchesAnActionFromANewThread_ThenThereShouldBeNoDeadlock()
+		{
+			Thread initialThread = Thread.CurrentThread;
+			Subject.SubscribeToAction<StoreInitializedAction>(this, _ =>
+			{
+				var thread = new Thread(() =>
+				{
+					Thread.Sleep(50);
+					while (Thread.CurrentThread == initialThread)
+						Thread.Sleep(0);
+
+					Dispatcher.Dispatch(new IncrementCounterAction());
+				});
+				thread.Start();
+				thread.Join();
+			});
+
+			var timeout = Task.Delay(1000);
+			var initialize = Task.Run(async () =>
+			{
+				await Task.Yield();
+				await Subject.InitializeAsync();
+			});
+			await Task.WhenAny(timeout, initialize);
+			Assert.False(timeout.IsCompleted, "Time out due to deadlock");
+		}
+
+		public DispatchReentrancyTests()
+		{
+			Dispatcher = new Dispatcher();
+			Subject = new Store(Dispatcher);
+
+			Feature = new CounterFeature();
+			Subject.AddFeature(Feature);
+		}
+
+	}
+}

Source/Tests/Fluxor.UnitTests/StoreTests/ThreadingTests/DispatchReentrancyTests/SupportFiles/CounterFeature.cs

@@ -0,0 +1,8 @@
+namespace Fluxor.UnitTests.StoreTests.ThreadingTests.DispatchReentrancyTests.SupportFiles
+{
+	class CounterFeature : Feature<CounterState>
+	{
+		public override string GetName() => "Counter";
+		protected override CounterState GetInitialState() => new(counter: 0);
+	}
+}

Source/Tests/Fluxor.UnitTests/StoreTests/ThreadingTests/DispatchReentrancyTests/SupportFiles/CounterState.cs

@@ -0,0 +1,13 @@
+namespace Fluxor.UnitTests.StoreTests.ThreadingTests.DispatchReentrancyTests.SupportFiles
+{
+	public class CounterState
+	{
+		public readonly int Counter;
+
+		public CounterState(int counter)
+		{
+			Counter = counter;
+		}
+	}
+}
+

Source/Tests/Fluxor.UnitTests/StoreTests/ThreadingTests/DispatchReentrancyTests/SupportFiles/IncrementCounterAction.cs

@@ -0,0 +1,6 @@
+namespace Fluxor.UnitTests.StoreTests.ThreadingTests.DispatchReentrancyTests.SupportFiles
+{
+	class IncrementCounterAction
+	{
+	}
+}

Source/Tests/Fluxor.UnitTests/StoreTests/ThreadingTests/DispatchReentrancyTests/SupportFiles/IsolatedTests.cs

@@ -0,0 +1,6 @@
+namespace Fluxor.UnitTests.StoreTests.ThreadingTests.DispatchReentrancyTests.SupportFiles
+{
+	public class IsolatedTests : Middleware
+	{
+	}
+}