Start thread pool worker threads in the default execution context

src/coreclr/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs

@@ -70,7 +70,7 @@ internal void ThreadStart(object? obj)
             _startArg = obj;
 
             ExecutionContext? context = _executionContext;
-            if (context != null)
+            if (context != null && !context.IsDefault)
             {
                 ExecutionContext.RunInternal(context, s_threadStartContextCallback, this);
             }
@@ -87,7 +87,7 @@ internal void ThreadStart()
             Debug.Assert(_start is ThreadStart);
 
             ExecutionContext? context = _executionContext;
-            if (context != null)
+            if (context != null && !context.IsDefault)
             {
                 ExecutionContext.RunInternal(context, s_threadStartContextCallback, this);
             }
@@ -212,9 +212,6 @@ public void Start()
             StartupSetApartmentStateInternal();
 #endif // FEATURE_COMINTEROP_APARTMENT_SUPPORT
 
-            // Attach current thread's security principal object to the new
-            // thread. Be careful not to bind the current thread to a principal
-            // if it's not already bound.
             if (_delegate != null)
             {
                 // If we reach here with a null delegate, something is broken. But we'll let the StartInternal method take care of
@@ -229,6 +226,29 @@ public void Start()
             StartInternal();
         }
 
+        [UnsupportedOSPlatform("browser")]
+        internal void UnsafeStart(object? parameter)
+        {
+            // We expect the thread to be setup with a ParameterizedThreadStart if this Start is called.
+            Debug.Assert(_delegate is ThreadStart);
+
+            _threadStartArg = parameter;
+            UnsafeStart();
+        }
+
+        [UnsupportedOSPlatform("browser")]
+        internal void UnsafeStart()
+        {
+#if FEATURE_COMINTEROP_APARTMENT_SUPPORT
+            // Eagerly initialize the COM Apartment state of the thread if we're allowed to.
+            StartupSetApartmentStateInternal();
+#endif // FEATURE_COMINTEROP_APARTMENT_SUPPORT
+
+            Debug.Assert(_delegate != null);
+            Debug.Assert(_delegate!.Target is ThreadHelper);
+            StartInternal();
+        }
+
         private void SetCultureOnUnstartedThreadNoCheck(CultureInfo value, bool uiCulture)
         {
             Debug.Assert(_delegate != null);

src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.GateThread.cs

@@ -168,11 +168,13 @@ private static void CreateGateThread(PortableThreadPool threadPoolInstance)
                 bool created = false;
                 try
                 {
+                    // Thread pool threads must start in the default execution context without transferring the context, so
+                    // using UnsafeStart() instead of Start()
                     Thread gateThread = new Thread(GateThreadStart, SmallStackSizeBytes);
                     gateThread.IsThreadPoolThread = true;
                     gateThread.IsBackground = true;
                     gateThread.Name = ".NET ThreadPool Gate";
-                    gateThread.Start();
+                    gateThread.UnsafeStart();
                     created = true;
                 }
                 finally

src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WaitThread.cs

@@ -183,11 +183,14 @@ internal class WaitThread
             public WaitThread()
             {
                 _waitHandles[0] = _changeHandlesEvent.SafeWaitHandle;
+
+                // Thread pool threads must start in the default execution context without transferring the context, so
+                // using UnsafeStart() instead of Start()
                 Thread waitThread = new Thread(WaitThreadStart, SmallStackSizeBytes);
                 waitThread.IsThreadPoolThread = true;
                 waitThread.IsBackground = true;
                 waitThread.Name = ".NET ThreadPool Wait";
-                waitThread.Start();
+                waitThread.UnsafeStart();
             }
 
             /// <summary>

src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.cs

@@ -288,10 +288,12 @@ private static bool TryCreateWorkerThread()
             {
                 try
                 {
+                    // Thread pool threads must start in the default execution context without transferring the context, so
+                    // using UnsafeStart() instead of Start()
                     Thread workerThread = new Thread(WorkerThreadStart);
                     workerThread.IsThreadPoolThread = true;
                     workerThread.IsBackground = true;
-                    workerThread.Start();
+                    workerThread.UnsafeStart();
                 }
                 catch (ThreadStartException)
                 {
@@ -301,6 +303,7 @@ private static bool TryCreateWorkerThread()
                 {
                     return false;
                 }
+
                 return true;
             }
         }

src/libraries/System.Threading.ThreadPool/tests/ThreadPoolTests.cs

@@ -835,6 +835,56 @@ public static void ThreadPoolCanProcessManyWorkItemsInParallelWithoutDeadlocking
             done.CheckedWait();
         }
 
+        [ThreadStatic]
+        private static int t_ThreadPoolThreadCreationDoesNotTransferExecutionContext_asyncLocalSideEffect;
+
+        [ConditionalFact(nameof(IsThreadingAndRemoteExecutorSupported))]
+        public static void ThreadPoolThreadCreationDoesNotTransferExecutionContext()
+        {
+            // Run in a separate process to test in a clean thread pool environment such that work items queued by the test
+            // would cause the thread pool to create threads
+            RemoteExecutor.Invoke(() =>
+            {
+                var done = new AutoResetEvent(false);
+
+                // Create an AsyncLocal with value change notifications, this changes the EC on this thread to non-default
+                var asyncLocal = new AsyncLocal<int>(e =>
+                {
+                    // There is nothing in this test that should cause a thread's EC to change due to EC flow
+                    Assert.False(e.ThreadContextChanged);
+
+                    // Record a side-effect from AsyncLocal value changes caused by flow. This is mainly because AsyncLocal
+                    // value change notifications can have side-effects like impersonation, we want to ensure that not only the
+                    // AsyncLocal's value is correct, but also that the side-effect matches the value, confirming that any value
+                    // changes cause matching notifications.
+                    t_ThreadPoolThreadCreationDoesNotTransferExecutionContext_asyncLocalSideEffect = e.CurrentValue;
+                });
+                asyncLocal.Value = 1;
+
+                ThreadPool.UnsafeQueueUserWorkItem(_ =>
+                {
+                    // The EC should not have flowed. If the EC had flowed, the assertion in the value change notification would
+                    // fail. Just for additional verification, check the side-effect as well.
+                    Assert.Equal(0, t_ThreadPoolThreadCreationDoesNotTransferExecutionContext_asyncLocalSideEffect);
+
+                    done.Set();
+                }, null);
+                done.CheckedWait();
+
+                ThreadPool.UnsafeRegisterWaitForSingleObject(done, (_, timedOut) =>
+                {
+                    Assert.True(timedOut);
+
+                    // The EC should not have flowed. If the EC had flowed, the assertion in the value change notification would
+                    // fail. Just for additional verification, check the side-effect as well.
+                    Assert.Equal(0, t_ThreadPoolThreadCreationDoesNotTransferExecutionContext_asyncLocalSideEffect);
+
+                    done.Set();
+                }, null, 0, true);
+                done.CheckedWait();
+            }).Dispose();
+        }
+
         public static bool IsThreadingAndRemoteExecutorSupported =>
             PlatformDetection.IsThreadingSupported && RemoteExecutor.IsSupported;
     }

src/libraries/System.Threading/tests/ExecutionContextTests.cs

@@ -237,14 +237,19 @@ private static void VerifyExecutionContextFlow(AsyncLocal<int> asyncLocal, int e
             }
             VerifyExecutionContext(ExecutionContext.Capture(), asyncLocal, expectedValue);
 
+            // Creating a thread flows context if and only if flow is not suppressed
             int asyncLocalValue = -1;
-            var done = new ManualResetEvent(false);
-            ThreadPool.QueueUserWorkItem(
-                state =>
-                {
-                    asyncLocalValue = asyncLocal.Value;
-                    done.Set();
-                });
+            ThreadTestHelpers.RunTestInBackgroundThread(() => asyncLocalValue = asyncLocal.Value);
+            Assert.Equal(expectedValue, asyncLocalValue);
+
+            // Queueing a thread pool work item flows context if and only if flow is not suppressed
+            asyncLocalValue = -1;
+            var done = new AutoResetEvent(false);
+            ThreadPool.QueueUserWorkItem(state =>
+            {
+                asyncLocalValue = asyncLocal.Value;
+                done.Set();
+            });
             done.CheckedWait();
             Assert.Equal(expectedValue, asyncLocalValue);
         }

src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Runtime.CompilerServices;
@@ -254,6 +255,7 @@ public static void Sleep(int millisecondsTimeout)
         [UnsupportedOSPlatform("browser")]
         public void Start()
         {
+            _executionContext = ExecutionContext.Capture();
             StartInternal(this);
         }
 
@@ -264,23 +266,69 @@ public void Start(object parameter)
                 throw new InvalidOperationException(SR.InvalidOperation_ThreadWrongThreadStart);
 
             m_start_arg = parameter;
+            Start();
+        }
+
+        [UnsupportedOSPlatform("browser")]
+        internal void UnsafeStart()
+        {
             StartInternal(this);
         }
 
+        [UnsupportedOSPlatform("browser")]
+        internal void UnsafeStart(object parameter)
+        {
+            Debug.Assert(m_start is ThreadStart);
+
+            m_start_arg = parameter;
+            UnsafeStart();
+        }
+
         // Called from the runtime
         internal void StartCallback()
+        {
+            ExecutionContext? context = _executionContext;
+            if (context != null && !context.IsDefault)
+            {
+                ExecutionContext.RunInternal(context, s_threadStartContextCallback, this);
+            }
+            else
+            {
+                StartCallbackWorker();
+            }
+        }
+
+        private static readonly ContextCallback s_threadStartContextCallback = new ContextCallback(StartCallback_Context);
+
+        private static void StartCallback_Context(object? state)
+        {
+            Debug.Assert(state is Thread);
+            ((Thread)state).StartCallbackWorker();
+        }
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)] // otherwise an unnecessary long-lived stack frame in many threads
+        private void StartCallbackWorker()
         {
             if (culture != null)
-                CurrentCulture = culture;
+            {
+                CultureInfo.CurrentCulture = culture;
+                culture = null;
+            }
+
             if (ui_culture != null)
-                CurrentUICulture = ui_culture;
+            {
+                CultureInfo.CurrentUICulture = ui_culture;
+                ui_culture = null;
+            }
+
             if (m_start is ThreadStart del)
             {
                 m_start = null;
                 del();
             }
             else
             {
+                Debug.Assert(m_start is ParameterizedThreadStart);
                 var pdel = (ParameterizedThreadStart)m_start!;
                 object? arg = m_start_arg;
                 m_start = null;