Merge pull request #25 from Timothy-LiuXuefeng/concurrent

fix: data race when used in concurrent environment
Timothy-Liuxf · Mar 25, 2022 · 3d70a85 · 3d70a85
2 parents 2389252 + 5911034
commit 3d70a85
Show file tree

Hide file tree

Showing 8 changed files with 199 additions and 66 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -22,4 +22,7 @@ jobs:
     - name: Build FrameRateTask
       run: dotnet build "./CSharp/FrameRateTask/FrameRateTask.csproj" -c Release
     - name: Run test code
-      run: dotnet build "./CSharp/Test/Test.csproj" -c Release
+      run: |
+        dotnet build "./CSharp/Test/Test.csproj" -c Release
+        dotnet build "./CSharp/TestConcurrency/TestConcurrency.csproj" -c Release
+        dotnet run --project "./CSharp/TestConcurrency/TestConcurrency.csproj" -c Release
diff --git a/CSharp/FrameRateTask/FrameRateTask.cs b/CSharp/FrameRateTask/FrameRateTask.cs
@@ -13,6 +13,7 @@
 
 namespace Timothy.FrameRateTask
 {
+
 	/// <summary>
 	/// The class intends to execute a task that need to be executed repeatedly every less than one second and need to be accurate.
 	/// </summary>
@@ -22,32 +23,41 @@ public class FrameRateTaskExecutor<TResult>
 		/// <summary>
 		/// The actual framerate recently.
 		/// </summary>
-		public uint FrameRate { get; private set; }
+		public uint FrameRate
+		{
+			get => (uint)Interlocked.CompareExchange(ref frameRate, 0, 0);
+			private set => Interlocked.Exchange(ref frameRate, value);
+		}
+		private long frameRate;
 
 		/// <summary>
 		/// Gets a value indicating whether or not the task has finished.
 		/// </summary>
 		/// <returns>
 		/// true if the task has finished; otherwise, false.
 		/// </returns>
-		public bool Finished { get; private set; } = false;
+		public bool Finished
+		{
+			get => Interlocked.CompareExchange(ref finished, 0, 0) != 0;
+			set => Interlocked.Exchange(ref finished, value ? 1 : 0);
+		}
+		private int finished = 0;
 
 		/// <summary>
 		/// Gets a value indicating whether or not the task has started.
 		/// </summary>
 		/// <returns>
 		/// true if the task has started; otherwise, false.
 		/// </returns>
-		public bool HasExecuted { get; private set; } = false;
-		private object hasExecutedLock = new object();
+		public bool HasExecuted { get => Interlocked.CompareExchange(ref hasExecuted, 0, 0) != 0; }
+		private int hasExecuted = 0;
 		private bool TrySetExecute()
 		{
-			lock (hasExecutedLock)
+			if (Interlocked.Exchange(ref hasExecuted, 1) != 0)
 			{
-				if (HasExecuted) return false;
-				HasExecuted = true;
-				return true;
+				return false;
 			}
+			return true;
 		}
 
 		private TResult result;
@@ -67,41 +77,54 @@ public TResult Result
 			private set => result = value;
 		}
 
-
 		/// <summary>
 		/// Gets or sets whether it allows time exceeding.
 		/// </summary>
 		/// <remarks>
 		/// If this property is false, the task will throw a TimeExceed exception when the task cannot finish in the given time.
 		/// The default value is true.
 		/// </remarks>
-		public bool AllowTimeExceed { get; set; } = true;
+		public bool AllowTimeExceed
+		{
+			get;
+#if NET5_0_OR_GREATER
+            init;
+#else
+			set;
+#endif
+		} = true;
 
 		/// <summary>
 		/// It will be called once time exceeds if AllowTimeExceed is set true.
 		/// </summary>
 		/// <remarks>
 		/// parameter bool: If it is called because of the number of time exceeding is greater than MaxTimeExceedCount, the argument is true; if it is called because of exceeding once, the argument is false.
 		/// </remarks>
-		public Action<bool> TimeExceedAction { get; set; } = callByExceed => { };
-
-		/// <summary>
-		/// The TickCount when beginning the loop,
-		/// </summary>
-		public long BeginTickCount { get; private set; } = 0L;
-
-		/// <summary>
-		/// The TickCount should be when ending last loop.
-		/// </summary>
-		public long LastLoopEndingTickCount { get; private set; }
+		public Action<bool> TimeExceedAction
+		{
+			get;
+#if NET5_0_OR_GREATER
+            init;
+#else
+			set;
+#endif
+		} = callByExceed => { };
 
 		/// <summary>
 		/// Gets or sets the maximum count of time exceeding continuously.
 		/// </summary>
 		/// <remarks>
 		/// The value is 5 for default.
 		/// </remarks>
-		public ulong MaxTolerantTimeExceedCount { get; set; } = 5;
+		public ulong MaxTolerantTimeExceedCount
+		{
+			get;
+#if NET5_0_OR_GREATER
+            init;
+#else
+			set;
+#endif
+		} = 5;
 
 		/// <summary>
 		/// Start this task synchronously.
@@ -155,12 +178,13 @@ public FrameRateTaskExecutor
 			loopFunc = () =>
 			{
 				ulong timeExceedCount = 0UL;
+				long lastLoopEndingTickCount, beginTickCount;
 
-				var nextTime = (LastLoopEndingTickCount = BeginTickCount = Environment.TickCount64) + timeInterval;
-				var endTime = BeginTickCount < long.MaxValue - maxTotalDuration ? BeginTickCount + maxTotalDuration : long.MaxValue;
+				var nextTime = (lastLoopEndingTickCount = beginTickCount = Environment.TickCount64) + timeInterval;
+				var endTime = beginTickCount < long.MaxValue - maxTotalDuration ? beginTickCount + maxTotalDuration : long.MaxValue;
 
 				uint loopCnt = 0;
-				var nextCntTime = BeginTickCount + 1000L;
+				var nextCntTime = beginTickCount + 1000L;
 
 				while (loopCondition() && nextTime <= endTime)
 				{
@@ -191,7 +215,7 @@ public FrameRateTaskExecutor
 						else if (AllowTimeExceed) TimeExceedAction(false);
 					}
 
-					LastLoopEndingTickCount = nextTime;
+					lastLoopEndingTickCount = nextTime;
 					nextTime += timeInterval;
 					++loopCnt;
 					if (Environment.TickCount64 >= nextCntTime)
@@ -203,9 +227,9 @@ public FrameRateTaskExecutor
 				}
 
 				result = finallyReturn();
+				Interlocked.MemoryBarrierProcessWide();
 				Finished = true;
 			};
-
 		}
 
 		/// <summary>

diff --git a/CSharp/FrameRateTask/FrameRateTask.csproj b/CSharp/FrameRateTask/FrameRateTask.csproj
@@ -2,8 +2,9 @@
 
   <PropertyGroup>
     <TargetFrameworks>netcoreapp3.1;net5.0;net6.0</TargetFrameworks>
-    <FileVersion>1.0.0.0</FileVersion>
-    <AssemblyVersion>1.0.0.0</AssemblyVersion>
+    <FileVersion>1.1.0.0</FileVersion>
+    <AssemblyVersion>1.1.0.0</AssemblyVersion>
+    <VersionPrefix>1.1.0</VersionPrefix>
     <RepositoryUrl>https://github.com/Timothy-LiuXuefeng/FrameRateTask</RepositoryUrl>
     <PackageReadmeFile>README.md</PackageReadmeFile>
     <Authors>TimothyLiuXuefeng</Authors>
@@ -13,6 +14,7 @@
     <PackageRequireLicenseAcceptance>True</PackageRequireLicenseAcceptance>
     <PackageLicenseFile>LICENSE.txt</PackageLicenseFile>
     <PackageProjectUrl>https://github.com/Timothy-LiuXuefeng/FrameRateTask</PackageProjectUrl>
+    <Nullable>disable</Nullable>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
@@ -23,6 +25,14 @@
     <DocumentationFile>.\FrameRateTask.xml</DocumentationFile>
   </PropertyGroup>
 
+  <PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|net6.0|AnyCPU'">
+    <WarningLevel>5</WarningLevel>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Release|net6.0|AnyCPU'">
+    <WarningLevel>5</WarningLevel>
+  </PropertyGroup>
+
   <ItemGroup>
     <None Include="..\..\README.md">
       <Pack>True</Pack>

diff --git a/CSharp/Test/Program.cs b/CSharp/Test/Program.cs
@@ -11,7 +11,7 @@ static void Demo1()			// The most common circumstance
 		{
 			Random r = new Random();
 			int cnt = 0;
-			FrameRateTaskExecutor<int> frt = new FrameRateTaskExecutor<int>
+			FrameRateTaskExecutor<int> executor = new FrameRateTaskExecutor<int>
 				(
 					() => true,
 					() =>
@@ -31,21 +31,21 @@ static void Demo1()			// The most common circumstance
 				(
 					() =>
 					{
-						while (!frt.Finished)
+						while (!executor.Finished)
 						{
-							Console.WriteLine($"Now framerate: { frt.FrameRate }");
+							Console.WriteLine($"Now framerate: { executor.FrameRate }");
 							Thread.Sleep(1000);
 						}
 					}
 				);
-			frt.Start();
+			executor.Start();
 		}
 
 		static void Demo2()			// The most common circumstance.
 		{
 			int i = 1, sum = 0;
 			Random r = new Random();
-			FrameRateTaskExecutor<int> frt = new FrameRateTaskExecutor<int>
+			FrameRateTaskExecutor<int> executor = new FrameRateTaskExecutor<int>
 				(
 					() => i <= 10,
 					() =>
@@ -64,8 +64,8 @@ static void Demo2()			// The most common circumstance.
 			{
 				MaxTolerantTimeExceedCount = ulong.MaxValue			// Set it to ulong.MaxValue in case time exceeding could cause the result incorrect.
 			};
-			frt.Start();
-			Console.WriteLine($"result: {frt.Result}");
+			executor.Start();
+			Console.WriteLine($"result: {executor.Result}");
 		}
 		static void Demo3()
 		{
@@ -109,7 +109,7 @@ static void Demo4()
 		{
 			Random r = new Random();
 			int tm = 0;
-			FrameRateTaskExecutor<int> frt = new FrameRateTaskExecutor<int>
+			FrameRateTaskExecutor<int> executor = new FrameRateTaskExecutor<int>
 				(
 					() => true,
 					() => 
@@ -127,14 +127,14 @@ static void Demo4()
 				(
 					() =>
 					{
-						while (!frt.Finished)
+						while (!executor.Finished)
 						{
-							Console.WriteLine($"Now framerate: { frt.FrameRate }");
+							Console.WriteLine($"Now framerate: { executor.FrameRate }");
 							Thread.Sleep(1000);
 						}
 					}
 				);
-			frt.Start();
+			executor.Start();
 		}
 
 		static void Demo5()
@@ -143,7 +143,7 @@ static void Demo5()
 				Random r = new Random();
 				int tm = 0;
 				int cnt = 0;
-				FrameRateTaskExecutor<int> frt = new FrameRateTaskExecutor<int>
+				FrameRateTaskExecutor<int> executor = new FrameRateTaskExecutor<int>
 					(
 						() => true,
 						() =>
@@ -166,14 +166,14 @@ static void Demo5()
 					(
 						() =>
 						{
-							while (!frt.Finished)
+							while (!executor.Finished)
 							{
-								Console.WriteLine($"Now framerate: { frt.FrameRate }");
+								Console.WriteLine($"Now framerate: { executor.FrameRate }");
 								Thread.Sleep(1000);
 							}
 						}
 					);
-				frt.Start();
+				executor.Start();
 			}
 			Console.WriteLine("You can see it is stable after it becomes clear.");
 
@@ -185,7 +185,7 @@ static void Demo5()
 				Random r = new Random();
 				int tm = 0;
 				int cnt = 0;
-				FrameRateTaskExecutor<int> frt = new FrameRateTaskExecutor<int>
+				FrameRateTaskExecutor<int> executor = new FrameRateTaskExecutor<int>
 					(
 						() => true,
 						() =>
@@ -208,14 +208,14 @@ static void Demo5()
 					(
 						() =>
 						{
-							while (!frt.Finished)
+							while (!executor.Finished)
 							{
-								Console.WriteLine($"Now framerate: { frt.FrameRate }");
+								Console.WriteLine($"Now framerate: { executor.FrameRate }");
 								Thread.Sleep(1000);
 							}
 						}
 					);
-				frt.Start();
+				executor.Start();
 				Console.WriteLine("You can see it is NOT stable after it becomes clear.");
 			}
 		}