Skip to content

Commit

Permalink
Add Thread.UnsafeStart (#47056)
Browse files Browse the repository at this point in the history
  • Loading branch information
stephentoub authored Jan 16, 2021
1 parent 3f23874 commit c7fda3b
Show file tree
Hide file tree
Showing 9 changed files with 127 additions and 55 deletions.
2 changes: 1 addition & 1 deletion eng/CodeAnalysis.ruleset
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@
<Rule Id="IDE0076" Action="Hidden" /> <!-- InvalidSuppressMessageAttribute -->
<Rule Id="IDE0077" Action="Hidden" /> <!-- LegacyFormatSuppressMessageAttribute -->
<Rule Id="IDE0078" Action="Hidden" /> <!-- UsePatternCombinators -->
<Rule Id="IDE0079" Action="Warning" /> <!-- RemoveUnnecessarySuppression -->
<Rule Id="IDE0079" Action="Hidden" /> <!-- RemoveUnnecessarySuppression -->
<Rule Id="IDE0080" Action="Hidden" /> <!-- RemoveConfusingSuppressionForIsExpression -->
<Rule Id="IDE0081" Action="Hidden" /> <!-- RemoveUnnecessaryByVal -->
<Rule Id="IDE0082" Action="Warning" /> <!-- ConvertTypeOfToNameOf -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,12 +199,12 @@ public static void ScheduleEventStream(SafeEventStreamHandle eventStream)
Debug.Assert(s_scheduledStreamsCount == 0);
s_scheduledStreamsCount = 1;
var runLoopStarted = new ManualResetEventSlim();
new Thread(args =>
new Thread(static args =>
{
object[] inputArgs = (object[])args!;
WatchForFileSystemEventsThreadStart((ManualResetEventSlim)inputArgs[0], (SafeEventStreamHandle)inputArgs[1]);
})
{ IsBackground = true }.Start(new object[] { runLoopStarted, eventStream });
{ IsBackground = true }.UnsafeStart(new object[] { runLoopStarted, eventStream });

runLoopStarted.Wait();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,20 +157,10 @@ private SocketAsyncEngine()
throw new InternalException(err);
}

bool suppressFlow = !ExecutionContext.IsFlowSuppressed();
try
{
if (suppressFlow) ExecutionContext.SuppressFlow();

Thread thread = new Thread(s => ((SocketAsyncEngine)s!).EventLoop());
thread.IsBackground = true;
thread.Name = ".NET Sockets";
thread.Start(this);
}
finally
{
if (suppressFlow) ExecutionContext.RestoreFlow();
}
var thread = new Thread(static s => ((SocketAsyncEngine)s!).EventLoop());
thread.IsBackground = true;
thread.Name = ".NET Sockets";
thread.UnsafeStart(this);
}
catch
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ private void EnableTimer(float pollingIntervalInSeconds)
ResetCounters(); // Reset statistics for counters before we start the thread.

_timeStampSinceCollectionStarted = DateTime.UtcNow;
#if ES_BUILD_STANDALONE
// Don't capture the current ExecutionContext and its AsyncLocals onto the timer causing them to live forever
bool restoreFlow = false;
try
Expand All @@ -142,7 +143,7 @@ private void EnableTimer(float pollingIntervalInSeconds)
ExecutionContext.SuppressFlow();
restoreFlow = true;
}

#endif
_nextPollingTimeStamp = DateTime.UtcNow + new TimeSpan(0, 0, (int)pollingIntervalInSeconds);

// Create the polling thread and init all the shared state if needed
Expand All @@ -151,7 +152,11 @@ private void EnableTimer(float pollingIntervalInSeconds)
s_pollingThreadSleepEvent = new AutoResetEvent(false);
s_counterGroupEnabledList = new List<CounterGroup>();
s_pollingThread = new Thread(PollForValues) { IsBackground = true };
#if ES_BUILD_STANDALONE
s_pollingThread.Start();
#else
s_pollingThread.UnsafeStart();
#endif
}

if (!s_counterGroupEnabledList!.Contains(this))
Expand All @@ -162,13 +167,15 @@ private void EnableTimer(float pollingIntervalInSeconds)
// notify the polling thread that the polling interval may have changed and the sleep should
// be recomputed
s_pollingThreadSleepEvent!.Set();
#if ES_BUILD_STANDALONE
}
finally
{
// Restore the current ExecutionContext
if (restoreFlow)
ExecutionContext.RestoreFlow();
}
#endif
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,14 @@ protected internal override void QueueTask(Task task)
if (Thread.IsThreadStartSupported && (options & TaskCreationOptions.LongRunning) != 0)
{
// Run LongRunning tasks on their own dedicated thread.
Thread thread = new Thread(s_longRunningThreadWork);
thread.IsBackground = true; // Keep this thread from blocking process shutdown
#if !TARGET_BROWSER
thread.Start(task);
#endif
#pragma warning disable CA1416 // TODO: https://github.com/dotnet/runtime/issues/44922
new Thread(s_longRunningThreadWork) { IsBackground = true }.UnsafeStart(task);
#pragma warning restore CA1416
}
else
{
// Normal handling for non-LongRunning tasks.
bool preferLocal = ((options & TaskCreationOptions.PreferFairness) == 0);
ThreadPool.UnsafeQueueUserWorkItemInternal(task, preferLocal);
ThreadPool.UnsafeQueueUserWorkItemInternal(task, (options & TaskCreationOptions.PreferFairness) == 0);
}
}

Expand Down
51 changes: 38 additions & 13 deletions src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,27 @@ public Thread(ParameterizedThreadStart start, int maxStackSize)
#if !TARGET_BROWSER
internal const bool IsThreadStartSupported = true;

/// <summary>Causes the operating system to change the state of the current instance to <see cref="ThreadState.Running"/>, and optionally supplies an object containing data to be used by the method the thread executes.</summary>
/// <param name="parameter">An object that contains data to be used by the method the thread executes.</param>
/// <exception cref="ThreadStateException">The thread has already been started.</exception>
/// <exception cref="OutOfMemoryException">There is not enough memory available to start this thread.</exception>
/// <exception cref="InvalidOperationException">This thread was created using a <see cref="ThreadStart"/> delegate instead of a <see cref="ParameterizedThreadStart"/> delegate.</exception>
[UnsupportedOSPlatform("browser")]
public void Start(object? parameter)
public void Start(object? parameter) => Start(parameter, captureContext: true);

/// <summary>Causes the operating system to change the state of the current instance to <see cref="ThreadState.Running"/>, and optionally supplies an object containing data to be used by the method the thread executes.</summary>
/// <param name="parameter">An object that contains data to be used by the method the thread executes.</param>
/// <exception cref="ThreadStateException">The thread has already been started.</exception>
/// <exception cref="OutOfMemoryException">There is not enough memory available to start this thread.</exception>
/// <exception cref="InvalidOperationException">This thread was created using a <see cref="ThreadStart"/> delegate instead of a <see cref="ParameterizedThreadStart"/> delegate.</exception>
/// <remarks>
/// Unlike <see cref="Start"/>, which captures the current <see cref="ExecutionContext"/> and uses that context to invoke the thread's delegate,
/// <see cref="UnsafeStart"/> explicitly avoids capturing the current context and flowing it to the invocation.
/// </remarks>
[UnsupportedOSPlatform("browser")]
public void UnsafeStart(object? parameter) => Start(parameter, captureContext: false);

private void Start(object? parameter, bool captureContext)
{
StartHelper? startHelper = _startHelper;

Expand All @@ -169,14 +188,29 @@ public void Start(object? parameter)
}

startHelper._startArg = parameter;
startHelper._executionContext = ExecutionContext.Capture();
startHelper._executionContext = captureContext ? ExecutionContext.Capture() : null;
}

StartCore();
}

/// <summary>Causes the operating system to change the state of the current instance to <see cref="ThreadState.Running"/>.</summary>
/// <exception cref="ThreadStateException">The thread has already been started.</exception>
/// <exception cref="OutOfMemoryException">There is not enough memory available to start this thread.</exception>
[UnsupportedOSPlatform("browser")]
public void Start() => Start(captureContext: true);

/// <summary>Causes the operating system to change the state of the current instance to <see cref="ThreadState.Running"/>.</summary>
/// <exception cref="ThreadStateException">The thread has already been started.</exception>
/// <exception cref="OutOfMemoryException">There is not enough memory available to start this thread.</exception>
/// <remarks>
/// Unlike <see cref="Start"/>, which captures the current <see cref="ExecutionContext"/> and uses that context to invoke the thread's delegate,
/// <see cref="UnsafeStart"/> explicitly avoids capturing the current context and flowing it to the invocation.
/// </remarks>
[UnsupportedOSPlatform("browser")]
public void Start()
public void UnsafeStart() => Start(captureContext: false);

private void Start(bool captureContext)
{
StartHelper? startHelper = _startHelper;

Expand All @@ -185,20 +219,11 @@ public void Start()
if (startHelper != null)
{
startHelper._startArg = null;
startHelper._executionContext = ExecutionContext.Capture();
startHelper._executionContext = captureContext ? ExecutionContext.Capture() : null;
}

StartCore();
}

internal void UnsafeStart()
{
Debug.Assert(_startHelper != null);
Debug.Assert(_startHelper._startArg == null);
Debug.Assert(_startHelper._executionContext == null);

StartCore();
}
#endif

private void RequireCurrentThread()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ public void Start(object? parameter) { }
[System.ObsoleteAttribute("Thread.Suspend has been deprecated. Please use other classes in System.Threading, such as Monitor, Mutex, Event, and Semaphore, to synchronize Threads or protect resources. https://go.microsoft.com/fwlink/?linkid=14202", false)]
public void Suspend() { }
public bool TrySetApartmentState(System.Threading.ApartmentState state) { throw null; }
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
public void UnsafeStart() { }
[System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")]
public void UnsafeStart(object? parameter) { }
public static byte VolatileRead(ref byte address) { throw null; }
public static double VolatileRead(ref double address) { throw null; }
public static short VolatileRead(ref short address) { throw null; }
Expand Down
77 changes: 60 additions & 17 deletions src/libraries/System.Threading.Thread/tests/ThreadTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1034,9 +1034,35 @@ public static void SleepTest()
Assert.InRange((int)stopwatch.ElapsedMilliseconds, 100, int.MaxValue);
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
public static void StartTest()
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
[InlineData(false)]
[InlineData(true)]
public static void StartTest(bool useUnsafeStart)
{
void Start(Thread t)
{
if (useUnsafeStart)
{
t.UnsafeStart();
}
else
{
t.Start();
}
}

void StartParameter(Thread t, object parameter)
{
if (useUnsafeStart)
{
t.UnsafeStart(parameter);
}
else
{
t.Start(parameter);
}
}

var e = new AutoResetEvent(false);
Action waitForThread;
Thread t = null;
Expand All @@ -1046,33 +1072,33 @@ public static void StartTest()
Assert.Same(t, Thread.CurrentThread);
});
t.IsBackground = true;
Assert.Throws<InvalidOperationException>(() => t.Start(null));
Assert.Throws<InvalidOperationException>(() => t.Start(t));
t.Start();
Assert.Throws<ThreadStateException>(() => t.Start());
Assert.Throws<InvalidOperationException>(() => StartParameter(t, null));
Assert.Throws<InvalidOperationException>(() => StartParameter(t, t));
Start(t);
Assert.Throws<ThreadStateException>(() => Start(t));
e.Set();
waitForThread();
Assert.Throws<ThreadStateException>(() => t.Start());
Assert.Throws<ThreadStateException>(() => Start(t));

t = ThreadTestHelpers.CreateGuardedThread(out waitForThread, parameter => e.CheckedWait());
t.IsBackground = true;
t.Start();
Assert.Throws<ThreadStateException>(() => t.Start());
Assert.Throws<ThreadStateException>(() => t.Start(null));
Assert.Throws<ThreadStateException>(() => t.Start(t));
Start(t);
Assert.Throws<ThreadStateException>(() => Start(t));
Assert.Throws<ThreadStateException>(() => StartParameter(t, null));
Assert.Throws<ThreadStateException>(() => StartParameter(t, t));
e.Set();
waitForThread();
Assert.Throws<ThreadStateException>(() => t.Start());
Assert.Throws<ThreadStateException>(() => t.Start(null));
Assert.Throws<ThreadStateException>(() => t.Start(t));
Assert.Throws<ThreadStateException>(() => Start(t));
Assert.Throws<ThreadStateException>(() => StartParameter(t, null));
Assert.Throws<ThreadStateException>(() => StartParameter(t, t));

t = ThreadTestHelpers.CreateGuardedThread(out waitForThread, parameter =>
{
Assert.Null(parameter);
Assert.Same(t, Thread.CurrentThread);
});
t.IsBackground = true;
t.Start();
Start(t);
waitForThread();

t = ThreadTestHelpers.CreateGuardedThread(out waitForThread, parameter =>
Expand All @@ -1081,7 +1107,7 @@ public static void StartTest()
Assert.Same(t, Thread.CurrentThread);
});
t.IsBackground = true;
t.Start(null);
StartParameter(t, null);
waitForThread();

t = ThreadTestHelpers.CreateGuardedThread(out waitForThread, parameter =>
Expand All @@ -1090,7 +1116,24 @@ public static void StartTest()
Assert.Same(t, Thread.CurrentThread);
});
t.IsBackground = true;
t.Start(t);
StartParameter(t, t);
waitForThread();

var al = new AsyncLocal<int>();
al.Value = 42;
t = ThreadTestHelpers.CreateGuardedThread(out waitForThread, parameter =>
{
if (useUnsafeStart)
{
Assert.Equal(0, al.Value);
}
else
{
Assert.Equal(42, al.Value);
}
});
t.IsBackground = true;
StartParameter(t, t);
waitForThread();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,11 @@ public partial class Thread

[UnsupportedOSPlatform("browser")]
public void Start(object parameter) => throw new PlatformNotSupportedException();

[UnsupportedOSPlatform("browser")]
public void UnsafeStart() => throw new PlatformNotSupportedException();

[UnsupportedOSPlatform("browser")]
public void UnsafeStart(object parameter) => throw new PlatformNotSupportedException();
}
}

0 comments on commit c7fda3b

Please sign in to comment.