Infrequent failure Mono Windows x64, ThreadPoolTests.QueueRegisterPositiveAndFlowTest

Only observed a couple of time in CoreFX, corlib-xunit Windows x64 Mono CI,
https://jenkins.mono-project.com/view/Mono/job/z/label=w64/8715/testReport/junit/Test%20collection%20for%20System.Threading.ThreadPools.Tests/ThreadPoolTests/System_Threading_ThreadPools_Tests_ThreadPoolTests_QueueRegisterPositiveAndFlowTest/

Test is using a local variable to set state between threads and very infrequent,
the reading thread seems to pick the old value.

Looks like coordination between reader/writer thread works as expected and since
this is an infrequent failure, doesn't look like a coordination issues or problem
with the ThreadPool (would have expected failures in many tests) implementation.

The test is however using a fragile construction using a local variable (backgroundAsyncLocalValue)
to share state between the writer/reader thread without any synchronization involved.
That could lead to infrequent failures due to memory read/write optimizations.

Since this is a shared local variable captured and shared between main thread and thread pool
it needs to have proper acquire/release semantics, or it could cause failures like these due
to memory optimization.

1 files changed, 5 insertions, 5 deletions

diff --git a/src/System.Threading.ThreadPool/tests/ThreadPoolTests.cs b/src/System.Threading.ThreadPool/tests/ThreadPoolTests.cs
index 988635fa91..34d01a3932 100644
--- a/src/System.Threading.ThreadPool/tests/ThreadPoolTests.cs
+++ b/src/System.Threading.ThreadPool/tests/ThreadPoolTests.cs
@@ -250,7 +250,7 @@ namespace System.Threading.ThreadPools.Tests
                             Assert.True(toUnregister.Unregister(threadDone));
                         }
                         test();
-                        backgroundAsyncLocalValue = asyncLocal.Value;
+                        Volatile.Write(ref backgroundAsyncLocalValue, asyncLocal.Value);
                     }
                     catch (Exception ex)
                     {
@@ -288,7 +288,7 @@ namespace System.Threading.ThreadPools.Tests
                 },
                 obj);
             waitForBackgroundWork(false);
-            Assert.Equal(1, backgroundAsyncLocalValue);
+            Assert.Equal(1, Volatile.Read(ref backgroundAsyncLocalValue));
 
             ThreadPool.UnsafeQueueUserWorkItem(
                 state =>
@@ -300,7 +300,7 @@ namespace System.Threading.ThreadPools.Tests
                 },
                 obj);
             waitForBackgroundWork(false);
-            Assert.Equal(0, backgroundAsyncLocalValue);
+            Assert.Equal(0, Volatile.Read(ref backgroundAsyncLocalValue));
 
             registeredWaitHandle =
                 ThreadPool.RegisterWaitForSingleObject(
@@ -317,7 +317,7 @@ namespace System.Threading.ThreadPools.Tests
                     UnexpectedTimeoutMilliseconds,
                     false);
             waitForBackgroundWork(true);
-            Assert.Equal(1, backgroundAsyncLocalValue);
+            Assert.Equal(1, Volatile.Read(ref backgroundAsyncLocalValue));
 
             registeredWaitHandle =
                 ThreadPool.UnsafeRegisterWaitForSingleObject(
@@ -334,7 +334,7 @@ namespace System.Threading.ThreadPools.Tests
                     UnexpectedTimeoutMilliseconds,
                     false);
             waitForBackgroundWork(true);
-            Assert.Equal(0, backgroundAsyncLocalValue);
+            Assert.Equal(0, Volatile.Read(ref backgroundAsyncLocalValue));
         }
 
         [Fact]