Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Thread.UnsafeStart #47056

Merged
merged 1 commit into from
Jan 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 });
stephentoub marked this conversation as resolved.
Show resolved Hide resolved

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());
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
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);
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
#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
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
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);
danmoseley marked this conversation as resolved.
Show resolved Hide resolved
}
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();
}
}