From 0c7781113cec74ed69aa6a38f9a3e845f40c3680 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Thu, 18 Oct 2018 06:01:49 -0400 Subject: [PATCH] Add public IThreadPoolWorkItem (#20387) - Changes the internal IThreadPoolWorkItem to be public, removing the legacy ThreadAbortException from it (which was specific to Task, anyway). - Removes the IThreadPoolWorkItem implementation from Task, so that devs can't write code like `ThreadPool.UnsafeQueueUserWorkItem(task);` or `((IThreadPoolWorkItem)task).Execute();`, both of which could end up doing a variety of bad things that could show up in a variety of ways, some discoverable, some less so. - Adds an internal UnsafeQueueUserWorkItemInternal that takes object so that it can be passed either an IThreadPoolUserWorkItem or a Task, - Changes the ThreadPool's queues to be in terms of object instead of IThreadPoolWorkItem - Changes the dispatch loop to type check for IThreadPoolWorkItem or Task so that both remain supported. --- .../Runtime/CompilerServices/YieldAwaitable.cs | 2 +- .../shared/System/Threading/SemaphoreSlim.cs | 18 +- .../Runtime/CompilerServices/AsyncMethodBuilder.cs | 6 +- .../src/System/Threading/Tasks/Task.cs | 34 ++-- .../src/System/Threading/Tasks/TaskContinuation.cs | 36 ++-- .../Threading/Tasks/ThreadPoolTaskScheduler.cs | 15 +- .../src/System/Threading/ThreadPool.cs | 189 ++++++++++++--------- .../src/System/Threading/Timer.cs | 7 +- 8 files changed, 146 insertions(+), 161 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Runtime/CompilerServices/YieldAwaitable.cs b/src/System.Private.CoreLib/shared/System/Runtime/CompilerServices/YieldAwaitable.cs index f160719..507ffac 100644 --- a/src/System.Private.CoreLib/shared/System/Runtime/CompilerServices/YieldAwaitable.cs +++ b/src/System.Private.CoreLib/shared/System/Runtime/CompilerServices/YieldAwaitable.cs @@ -143,7 +143,7 @@ namespace System.Runtime.CompilerServices TaskScheduler scheduler = TaskScheduler.Current; if (scheduler == TaskScheduler.Default) { - ThreadPool.UnsafeQueueCustomWorkItem(box, forceGlobal: true); + ThreadPool.UnsafeQueueUserWorkItemInternal(box, preferLocal: false); } else { diff --git a/src/System.Private.CoreLib/shared/System/Threading/SemaphoreSlim.cs b/src/System.Private.CoreLib/shared/System/Threading/SemaphoreSlim.cs index a8c4984..40925dd 100644 --- a/src/System.Private.CoreLib/shared/System/Threading/SemaphoreSlim.cs +++ b/src/System.Private.CoreLib/shared/System/Threading/SemaphoreSlim.cs @@ -75,19 +75,16 @@ namespace System.Threading private const int NO_MAXIMUM = int.MaxValue; // Task in a linked list of asynchronous waiters - private sealed class TaskNode : Task, IThreadPoolWorkItem + private sealed class TaskNode : Task { internal TaskNode Prev, Next; internal TaskNode() : base() { } - void IThreadPoolWorkItem.ExecuteWorkItem() + internal override void ExecuteFromThreadPool() { bool setSuccessfully = TrySetResult(true); Debug.Assert(setSuccessfully, "Should have been able to complete task"); } -#if CORECLR - void IThreadPoolWorkItem.MarkAborted(ThreadAbortException tae) { /* nop */ } -#endif } #endregion @@ -851,7 +848,7 @@ namespace System.Threading // Get the next async waiter to release and queue it to be completed var waiterTask = m_asyncHead; RemoveAsyncWaiter(waiterTask); // ensures waiterTask.Next/Prev are null - QueueWaiterTask(waiterTask); + ThreadPool.UnsafeQueueUserWorkItemInternal(waiterTask, preferLocal: true); } } m_currentCount = currentCount; @@ -868,15 +865,6 @@ namespace System.Threading } /// - /// Queues a waiter task to the ThreadPool. We use this small helper method so that - /// the larger Release(count) method does not need to be SecuritySafeCritical. - /// - private static void QueueWaiterTask(TaskNode waiterTask) - { - ThreadPool.UnsafeQueueCustomWorkItem(waiterTask, forceGlobal: false); - } - - /// /// Releases all resources used by the current instance of . /// diff --git a/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilder.cs b/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilder.cs index acfbd14..fc40c92 100644 --- a/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilder.cs +++ b/src/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncMethodBuilder.cs @@ -540,9 +540,7 @@ namespace System.Runtime.CompilerServices /// A delegate to the method. public Action MoveNextAction => _moveNextAction ?? (_moveNextAction = new Action(MoveNext)); - /// Invokes when the box is queued to the thread pool and executed as a work item. - void IThreadPoolWorkItem.ExecuteWorkItem() => MoveNext(); - void IThreadPoolWorkItem.MarkAborted(ThreadAbortException tae) { /* nop */ } + internal sealed override void ExecuteFromThreadPool() => MoveNext(); /// Calls MoveNext on public void MoveNext() @@ -905,7 +903,7 @@ namespace System.Runtime.CompilerServices /// /// An interface implemented by all instances, regardless of generics. /// - internal interface IAsyncStateMachineBox : IThreadPoolWorkItem + internal interface IAsyncStateMachineBox { /// Move the state machine forward. void MoveNext(); diff --git a/src/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs b/src/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs index d6e4103..2adef9c 100644 --- a/src/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs +++ b/src/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs @@ -130,7 +130,7 @@ namespace System.Threading.Tasks /// [DebuggerTypeProxy(typeof(SystemThreadingTasks_TaskDebugView))] [DebuggerDisplay("Id = {Id}, Status = {Status}, Method = {DebuggerDisplayMethodDescription}")] - public class Task : IThreadPoolWorkItem, IAsyncResult, IDisposable + public class Task : IAsyncResult, IDisposable { [ThreadStatic] internal static Task t_currentTask; // The currently executing task. @@ -2317,19 +2317,10 @@ namespace System.Threading.Tasks } /// - /// IThreadPoolWorkItem override, which is the entry function for this task when the TP scheduler decides to run it. - /// - /// - void IThreadPoolWorkItem.ExecuteWorkItem() - { - ExecuteEntryUnsafe(); - } - - /// - /// The ThreadPool calls this if a ThreadAbortException is thrown while trying to execute this workitem. This may occur - /// before Task would otherwise be able to observe it. + /// The ThreadPool calls this if a ThreadAbortException is thrown while trying to execute this Task. + /// This may occur before Task would otherwise be able to observe it. /// - void IThreadPoolWorkItem.MarkAborted(ThreadAbortException tae) + internal virtual void MarkAbortedFromThreadPool(ThreadAbortException tae) { // If the task has marked itself as Completed, then it either a) already observed this exception (so we shouldn't handle it here) // or b) completed before the exception ocurred (in which case it shouldn't count against this Task). @@ -2370,6 +2361,14 @@ namespace System.Threading.Tasks return true; } + /// + /// ThreadPool's entry point into the Task. The base behavior is simply to + /// use the entry point that's not protected from double-invoke; derived internal tasks + /// can override to customize their behavior, which is usually done by promises + /// that want to reuse the same object as a queued work item. + /// + internal virtual void ExecuteFromThreadPool() => ExecuteEntryUnsafe(); + internal void ExecuteEntryUnsafe() // used instead of ExecuteEntry() when we don't have to worry about double-execution prevent { // Remember that we started running the task delegate. @@ -3373,7 +3372,7 @@ namespace System.Threading.Tasks } else { - ThreadPool.UnsafeQueueCustomWorkItem(new CompletionActionInvoker(completionAction, this), forceGlobal: false); + ThreadPool.UnsafeQueueUserWorkItemInternal(new CompletionActionInvoker(completionAction, this), preferLocal: true); } } @@ -6211,15 +6210,10 @@ namespace System.Threading.Tasks m_completingTask = completingTask; } - void IThreadPoolWorkItem.ExecuteWorkItem() + void IThreadPoolWorkItem.Execute() { m_action.Invoke(m_completingTask); } - - void IThreadPoolWorkItem.MarkAborted(ThreadAbortException tae) - { - /* NOP */ - } } // Proxy class for better debugging experience diff --git a/src/System.Private.CoreLib/src/System/Threading/Tasks/TaskContinuation.cs b/src/System.Private.CoreLib/src/System/Threading/Tasks/TaskContinuation.cs index 7abe63c..ab58595 100644 --- a/src/System.Private.CoreLib/src/System/Threading/Tasks/TaskContinuation.cs +++ b/src/System.Private.CoreLib/src/System/Threading/Tasks/TaskContinuation.cs @@ -588,7 +588,7 @@ namespace System.Threading.Tasks } // We couldn't inline, so now we need to schedule it - ThreadPool.UnsafeQueueCustomWorkItem(this, forceGlobal: false); + ThreadPool.UnsafeQueueUserWorkItemInternal(this, preferLocal: true); } } @@ -624,10 +624,17 @@ namespace System.Threading.Tasks } } - /// IThreadPoolWorkItem override, which is the entry function for this when the ThreadPool scheduler decides to run it. - private void ExecuteWorkItemHelper() + void IThreadPoolWorkItem.Execute() { var etwLog = TplEtwProvider.Log; + ExecutionContext context = m_capturedContext; + + if (!etwLog.IsEnabled() && context == null) + { + m_action(); + return; + } + Guid savedActivityId = Guid.Empty; if (etwLog.TasksSetActivityIds && m_continuationId != 0) { @@ -640,7 +647,6 @@ namespace System.Threading.Tasks // We're on a thread pool thread with no higher-level callers, so exceptions can just propagate. // If there's no execution context, just invoke the delegate. - ExecutionContext context = m_capturedContext; if (context == null) { m_action(); @@ -660,24 +666,6 @@ namespace System.Threading.Tasks } } - void IThreadPoolWorkItem.ExecuteWorkItem() - { - // inline the fast path - if (m_capturedContext == null && !TplEtwProvider.Log.IsEnabled()) - { - m_action(); - } - else - { - ExecuteWorkItemHelper(); - } - } - - /// - /// The ThreadPool calls this if a ThreadAbortException is thrown while trying to execute this workitem. - /// - void IThreadPoolWorkItem.MarkAborted(ThreadAbortException tae) { /* nop */ } - /// Cached delegate that invokes an Action passed as an object parameter. private static ContextCallback s_invokeActionCallback; @@ -802,7 +790,7 @@ namespace System.Threading.Tasks } else { - ThreadPool.UnsafeQueueCustomWorkItem(box, forceGlobal: false); + ThreadPool.UnsafeQueueUserWorkItemInternal(box, preferLocal: true); } return; } @@ -836,7 +824,7 @@ namespace System.Threading.Tasks etwLog.AwaitTaskContinuationScheduled((task.ExecutingTaskScheduler ?? TaskScheduler.Default).Id, task.Id, atc.m_continuationId); } - ThreadPool.UnsafeQueueCustomWorkItem(atc, forceGlobal: false); + ThreadPool.UnsafeQueueUserWorkItemInternal(atc, preferLocal: true); } /// Throws the exception asynchronously on the ThreadPool. diff --git a/src/System.Private.CoreLib/src/System/Threading/Tasks/ThreadPoolTaskScheduler.cs b/src/System.Private.CoreLib/src/System/Threading/Tasks/ThreadPoolTaskScheduler.cs index 87d6d7b..93aa26c 100644 --- a/src/System.Private.CoreLib/src/System/Threading/Tasks/ThreadPoolTaskScheduler.cs +++ b/src/System.Private.CoreLib/src/System/Threading/Tasks/ThreadPoolTaskScheduler.cs @@ -41,7 +41,8 @@ namespace System.Threading.Tasks /// The task to schedule. protected internal override void QueueTask(Task task) { - if ((task.Options & TaskCreationOptions.LongRunning) != 0) + TaskCreationOptions options = task.Options; + if ((options & TaskCreationOptions.LongRunning) != 0) { // Run LongRunning tasks on their own dedicated thread. Thread thread = new Thread(s_longRunningThreadWork); @@ -51,8 +52,8 @@ namespace System.Threading.Tasks else { // Normal handling for non-LongRunning tasks. - bool forceToGlobalQueue = ((task.Options & TaskCreationOptions.PreferFairness) != 0); - ThreadPool.UnsafeQueueCustomWorkItem(task, forceToGlobalQueue); + bool preferLocal = ((options & TaskCreationOptions.PreferFairness) == 0); + ThreadPool.UnsafeQueueUserWorkItemInternal(task, preferLocal); } } @@ -94,13 +95,13 @@ namespace System.Threading.Tasks return FilterTasksFromWorkItems(ThreadPool.GetQueuedWorkItems()); } - private IEnumerable FilterTasksFromWorkItems(IEnumerable tpwItems) + private IEnumerable FilterTasksFromWorkItems(IEnumerable tpwItems) { - foreach (IThreadPoolWorkItem tpwi in tpwItems) + foreach (object tpwi in tpwItems) { - if (tpwi is Task) + if (tpwi is Task t) { - yield return (Task)tpwi; + yield return t; } } } diff --git a/src/System.Private.CoreLib/src/System/Threading/ThreadPool.cs b/src/System.Private.CoreLib/src/System/Threading/ThreadPool.cs index d9b378a..ee917cf 100644 --- a/src/System.Private.CoreLib/src/System/Threading/ThreadPool.cs +++ b/src/System.Private.CoreLib/src/System/Threading/ThreadPool.cs @@ -19,7 +19,8 @@ using System.Diagnostics.Tracing; using System.Runtime.CompilerServices; using System.Runtime.ConstrainedExecution; using System.Runtime.InteropServices; -using System.Security; +using System.Threading.Tasks; +using Internal.Runtime.CompilerServices; using Microsoft.Win32; namespace System.Threading @@ -109,7 +110,7 @@ namespace System.Threading internal sealed class WorkStealingQueue { private const int INITIAL_SIZE = 32; - internal volatile IThreadPoolWorkItem[] m_array = new IThreadPoolWorkItem[INITIAL_SIZE]; + internal volatile object[] m_array = new object[INITIAL_SIZE]; private volatile int m_mask = INITIAL_SIZE - 1; #if DEBUG @@ -124,7 +125,7 @@ namespace System.Threading private SpinLock m_foreignLock = new SpinLock(enableThreadOwnerTracking: false); - public void LocalPush(IThreadPoolWorkItem obj) + public void LocalPush(object obj) { int tail = m_tailIndex; @@ -181,7 +182,7 @@ namespace System.Threading if (count >= m_mask) { // We're full; expand the queue by doubling its size. - var newArray = new IThreadPoolWorkItem[m_array.Length << 1]; + var newArray = new object[m_array.Length << 1]; for (int i = 0; i < m_array.Length; i++) newArray[i] = m_array[(i + head) & m_mask]; @@ -204,12 +205,12 @@ namespace System.Threading } [SuppressMessage("Microsoft.Concurrency", "CA8001", Justification = "Reviewed for thread safety")] - public bool LocalFindAndPop(IThreadPoolWorkItem obj) + public bool LocalFindAndPop(object obj) { // Fast path: check the tail. If equal, we can skip the lock. if (m_array[(m_tailIndex - 1) & m_mask] == obj) { - IThreadPoolWorkItem unused = LocalPop(); + object unused = LocalPop(); Debug.Assert(unused == null || unused == obj); return unused != null; } @@ -260,10 +261,10 @@ namespace System.Threading return false; } - public IThreadPoolWorkItem LocalPop() => m_headIndex < m_tailIndex ? LocalPopCore() : null; + public object LocalPop() => m_headIndex < m_tailIndex ? LocalPopCore() : null; [SuppressMessage("Microsoft.Concurrency", "CA8001", Justification = "Reviewed for thread safety")] - private IThreadPoolWorkItem LocalPopCore() + private object LocalPopCore() { while (true) { @@ -281,7 +282,7 @@ namespace System.Threading if (m_headIndex <= tail) { int idx = tail & m_mask; - IThreadPoolWorkItem obj = Volatile.Read(ref m_array[idx]); + object obj = Volatile.Read(ref m_array[idx]); // Check for nulls in the array. if (obj == null) continue; @@ -301,7 +302,7 @@ namespace System.Threading { // Element still available. Take it. int idx = tail & m_mask; - IThreadPoolWorkItem obj = Volatile.Read(ref m_array[idx]); + object obj = Volatile.Read(ref m_array[idx]); // Check for nulls in the array. if (obj == null) continue; @@ -327,7 +328,7 @@ namespace System.Threading public bool CanSteal => m_headIndex < m_tailIndex; - public IThreadPoolWorkItem TrySteal(ref bool missedSteal) + public object TrySteal(ref bool missedSteal) { while (true) { @@ -346,7 +347,7 @@ namespace System.Threading if (head < m_tailIndex) { int idx = head & m_mask; - IThreadPoolWorkItem obj = Volatile.Read(ref m_array[idx]); + object obj = Volatile.Read(ref m_array[idx]); // Check for nulls in the array. if (obj == null) continue; @@ -376,7 +377,7 @@ namespace System.Threading } internal bool loggingEnabled; - internal readonly ConcurrentQueue workItems = new ConcurrentQueue(); + internal readonly ConcurrentQueue workItems = new ConcurrentQueue(); private Internal.PaddingFor32 pad1; @@ -435,8 +436,10 @@ namespace System.Threading } } - public void Enqueue(IThreadPoolWorkItem callback, bool forceGlobal) + public void Enqueue(object callback, bool forceGlobal) { + Debug.Assert((callback is IThreadPoolWorkItem) ^ (callback is Task)); + if (loggingEnabled) System.Diagnostics.Tracing.FrameworkEventSource.Log.ThreadPoolEnqueueWorkObject(callback); @@ -456,16 +459,16 @@ namespace System.Threading EnsureThreadRequested(); } - internal bool LocalFindAndPop(IThreadPoolWorkItem callback) + internal bool LocalFindAndPop(object callback) { ThreadPoolWorkQueueThreadLocals tl = ThreadPoolWorkQueueThreadLocals.threadLocals; return tl != null && tl.workStealingQueue.LocalFindAndPop(callback); } - public IThreadPoolWorkItem Dequeue(ThreadPoolWorkQueueThreadLocals tl, ref bool missedSteal) + public object Dequeue(ThreadPoolWorkQueueThreadLocals tl, ref bool missedSteal) { WorkStealingQueue localWsq = tl.workStealingQueue; - IThreadPoolWorkItem callback; + object callback; if ((callback = localWsq.LocalPop()) == null && // first try the local queue !workItems.TryDequeue(out callback)) // then try the global queue @@ -522,7 +525,7 @@ namespace System.Threading // false later, but only if we're absolutely certain that the queue is empty. // bool needAnotherThread = true; - IThreadPoolWorkItem workItem = null; + object workItem = null; try { // @@ -573,7 +576,15 @@ namespace System.Threading { ThreadPool.ReportThreadStatus(isWorking: true); reportedStatus = true; - workItem.ExecuteWorkItem(); + if (workItem is Task task) + { + task.ExecuteFromThreadPool(); + } + else + { + Debug.Assert(workItem is IThreadPoolWorkItem); + Unsafe.As(workItem).Execute(); + } } finally { @@ -581,9 +592,18 @@ namespace System.Threading ThreadPool.ReportThreadStatus(isWorking: false); } } + else if (workItem is Task task) + { + // Check for Task first as it's currently faster to type check + // for Task and then Unsafe.As for the interface, rather than + // vice versa, in particular when the object implements a bunch + // of interfaces. + task.ExecuteFromThreadPool(); + } else { - workItem.ExecuteWorkItem(); + Debug.Assert(workItem is IThreadPoolWorkItem); + Unsafe.As(workItem).Execute(); } workItem = null; @@ -606,13 +626,15 @@ namespace System.Threading // it was executed or not (in debug builds only). Task uses this to communicate the ThreadAbortException to anyone // who waits for the task to complete. // - workItem?.MarkAborted(tae); + if (workItem is Task task) + { + task.MarkAbortedFromThreadPool(tae); + } // // In this case, the VM is going to request another thread on our behalf. No need to do it twice. // needAnotherThread = false; - // throw; //no need to explicitly rethrow a ThreadAbortException, and doing so causes allocations on amd64. } finally { @@ -678,7 +700,7 @@ namespace System.Threading { if (null != workQueue) { - IThreadPoolWorkItem cb; + object cb; while ((cb = workStealingQueue.LocalPop()) != null) { Debug.Assert(null != cb); @@ -868,6 +890,12 @@ namespace System.Threading public delegate void WaitOrTimerCallback(object state, bool timedOut); // signaled or timed out + /// Represents a work item that can be executed by the ThreadPool. + public interface IThreadPoolWorkItem + { + void Execute(); + } + // // This type is necessary because VS 2010's debugger looks for a method named _ThreadPoolWaitCallbacck.PerformWaitCallback // on the stack to determine if a thread is a ThreadPool thread or not. We have a better way to do this for .NET 4.5, but @@ -879,21 +907,6 @@ namespace System.Threading internal static bool PerformWaitCallback() => ThreadPoolWorkQueue.Dispatch(); } - // - // Interface to something that can be queued to the TP. This is implemented by - // QueueUserWorkItemCallback, Task, and potentially other internal types. - // For example, SemaphoreSlim represents callbacks using its own type that - // implements IThreadPoolWorkItem. - // - // If we decide to expose some of the workstealing - // stuff, this is NOT the thing we want to expose to the public. - // - internal interface IThreadPoolWorkItem - { - void ExecuteWorkItem(); - void MarkAborted(ThreadAbortException tae); - } - internal abstract class QueueUserWorkItemCallbackBase : IThreadPoolWorkItem { #if DEBUG @@ -905,29 +918,15 @@ namespace System.Threading executed != 0 || Environment.HasShutdownStarted, "A QueueUserWorkItemCallback was never called!"); } +#endif - protected void MarkExecuted(bool aborted) + public virtual void Execute() { +#if DEBUG GC.SuppressFinalize(this); Debug.Assert( - 0 == Interlocked.Exchange(ref executed, 1) || aborted, + 0 == Interlocked.Exchange(ref executed, 1), "A QueueUserWorkItemCallback was called twice!"); - } -#endif - - void IThreadPoolWorkItem.MarkAborted(ThreadAbortException tae) - { -#if DEBUG - // This workitem didn't execute because we got a ThreadAbortException prior to the call to ExecuteWorkItem. - // This counts as being executed for our purposes. - MarkExecuted(aborted: true); -#endif - } - - public virtual void ExecuteWorkItem() - { -#if DEBUG - MarkExecuted(aborted: false); #endif } } @@ -954,9 +953,9 @@ namespace System.Threading _context = context; } - public override void ExecuteWorkItem() + public override void Execute() { - base.ExecuteWorkItem(); + base.Execute(); ExecutionContext context = _context; if (context == null) { @@ -993,9 +992,9 @@ namespace System.Threading _context = context; } - public override void ExecuteWorkItem() + public override void Execute() { - base.ExecuteWorkItem(); + base.Execute(); ExecutionContext context = _context; if (context == null) { @@ -1030,9 +1029,9 @@ namespace System.Threading _state = state; } - public override void ExecuteWorkItem() + public override void Execute() { - base.ExecuteWorkItem(); + base.Execute(); ExecutionContext.RunInternal(executionContext: null, s_executionContextShim, this); // null executionContext on RunInternal is Default context } } @@ -1057,9 +1056,9 @@ namespace System.Threading _state = state; } - public override void ExecuteWorkItem() + public override void Execute() { - base.ExecuteWorkItem(); + base.Execute(); ExecutionContext.RunInternal(executionContext: null, s_executionContextShim, this); // null executionContext on RunInternal is Default context } } @@ -1304,9 +1303,9 @@ namespace System.Threading ExecutionContext context = ExecutionContext.Capture(); - IThreadPoolWorkItem tpcallBack = (context != null && context.IsDefault) ? + object tpcallBack = (context != null && context.IsDefault) ? new QueueUserWorkItemCallbackDefaultContext(callBack, state) : - (IThreadPoolWorkItem)new QueueUserWorkItemCallback(callBack, state, context); + (object)new QueueUserWorkItemCallback(callBack, state, context); ThreadPoolGlobals.workQueue.Enqueue(tpcallBack, forceGlobal: true); @@ -1324,9 +1323,9 @@ namespace System.Threading ExecutionContext context = ExecutionContext.Capture(); - IThreadPoolWorkItem tpcallBack = (context != null && context.IsDefault) ? + object tpcallBack = (context != null && context.IsDefault) ? new QueueUserWorkItemCallbackDefaultContext(callBack, state) : - (IThreadPoolWorkItem)new QueueUserWorkItemCallback(callBack, state, context); + (object)new QueueUserWorkItemCallback(callBack, state, context); ThreadPoolGlobals.workQueue.Enqueue(tpcallBack, forceGlobal: !preferLocal); @@ -1342,22 +1341,41 @@ namespace System.Threading EnsureVMInitialized(); - IThreadPoolWorkItem tpcallBack = new QueueUserWorkItemCallback(callBack, state, null); + object tpcallBack = new QueueUserWorkItemCallback(callBack, state, null); ThreadPoolGlobals.workQueue.Enqueue(tpcallBack, forceGlobal: true); return true; } - internal static void UnsafeQueueCustomWorkItem(IThreadPoolWorkItem workItem, bool forceGlobal) + public static bool UnsafeQueueUserWorkItem(IThreadPoolWorkItem callBack, bool preferLocal) { - Debug.Assert(null != workItem); + if (callBack == null) + { + ThrowHelper.ThrowArgumentNullException(ExceptionArgument.callBack); + } + if (callBack is Task) + { + // Prevent code from queueing a derived Task that also implements the interface, + // as that would bypass Task.Start and its safety checks. + ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.callBack); + } + + UnsafeQueueUserWorkItemInternal(callBack, preferLocal); + return true; + } + + internal static void UnsafeQueueUserWorkItemInternal(object callBack, bool preferLocal) + { + Debug.Assert((callBack is IThreadPoolWorkItem) ^ (callBack is Task)); + EnsureVMInitialized(); - ThreadPoolGlobals.workQueue.Enqueue(workItem, forceGlobal); + + ThreadPoolGlobals.workQueue.Enqueue(callBack, forceGlobal: !preferLocal); } // This method tries to take the target callback out of the current thread's queue. - internal static bool TryPopCustomWorkItem(IThreadPoolWorkItem workItem) + internal static bool TryPopCustomWorkItem(object workItem) { Debug.Assert(null != workItem); return @@ -1366,10 +1384,10 @@ namespace System.Threading } // Get all workitems. Called by TaskScheduler in its debugger hooks. - internal static IEnumerable GetQueuedWorkItems() + internal static IEnumerable GetQueuedWorkItems() { // Enumerate global queue - foreach (IThreadPoolWorkItem workItem in ThreadPoolGlobals.workQueue.workItems) + foreach (object workItem in ThreadPoolGlobals.workQueue.workItems) { yield return workItem; } @@ -1379,10 +1397,10 @@ namespace System.Threading { if (wsq != null && wsq.m_array != null) { - IThreadPoolWorkItem[] items = wsq.m_array; + object[] items = wsq.m_array; for (int i = 0; i < items.Length; i++) { - IThreadPoolWorkItem item = items[i]; + object item = items[i]; if (item != null) { yield return item; @@ -1392,34 +1410,34 @@ namespace System.Threading } } - internal static IEnumerable GetLocallyQueuedWorkItems() + internal static IEnumerable GetLocallyQueuedWorkItems() { ThreadPoolWorkQueue.WorkStealingQueue wsq = ThreadPoolWorkQueueThreadLocals.threadLocals.workStealingQueue; if (wsq != null && wsq.m_array != null) { - IThreadPoolWorkItem[] items = wsq.m_array; + object[] items = wsq.m_array; for (int i = 0; i < items.Length; i++) { - IThreadPoolWorkItem item = items[i]; + object item = items[i]; if (item != null) yield return item; } } } - internal static IEnumerable GetGloballyQueuedWorkItems() => ThreadPoolGlobals.workQueue.workItems; + internal static IEnumerable GetGloballyQueuedWorkItems() => ThreadPoolGlobals.workQueue.workItems; - private static object[] ToObjectArray(IEnumerable workitems) + private static object[] ToObjectArray(IEnumerable workitems) { int i = 0; - foreach (IThreadPoolWorkItem item in workitems) + foreach (object item in workitems) { i++; } object[] result = new object[i]; i = 0; - foreach (IThreadPoolWorkItem item in workitems) + foreach (object item in workitems) { if (i < result.Length) //just in case someone calls us while the queues are in motion result[i] = item; @@ -1462,9 +1480,10 @@ namespace System.Threading } } + [MethodImpl(MethodImplOptions.NoInlining)] private static void EnsureVMInitializedCore() { - ThreadPool.InitializeVMTp(ref ThreadPoolGlobals.enableWorkerTracking); + InitializeVMTp(ref ThreadPoolGlobals.enableWorkerTracking); ThreadPoolGlobals.vmTpInitialized = true; } diff --git a/src/System.Private.CoreLib/src/System/Threading/Timer.cs b/src/System.Private.CoreLib/src/System/Threading/Timer.cs index 67516de..efea457 100644 --- a/src/System.Private.CoreLib/src/System/Threading/Timer.cs +++ b/src/System.Private.CoreLib/src/System/Threading/Timer.cs @@ -314,7 +314,7 @@ namespace System.Threading } else { - ThreadPool.UnsafeQueueCustomWorkItem(timer, forceGlobal: true); + ThreadPool.UnsafeQueueUserWorkItemInternal(timer, preferLocal: false); } } else @@ -619,6 +619,7 @@ namespace System.Threading return success; } + void IThreadPoolWorkItem.Execute() => Fire(); internal void Fire() { @@ -648,10 +649,6 @@ namespace System.Threading SignalNoCallbacksRunning(); } - void IThreadPoolWorkItem.ExecuteWorkItem() => Fire(); - - void IThreadPoolWorkItem.MarkAborted(ThreadAbortException tae) { } - internal void SignalNoCallbacksRunning() { Interop.Kernel32.SetEvent(m_notifyWhenNoCallbacksRunning.SafeWaitHandle); -- 2.7.4