From 3edf2cd5755d5bf2daa045fa0e05e75ef5410791 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Lema=C5=84czyk?= Date: Tue, 28 Oct 2025 14:29:34 +0100 Subject: [PATCH] Stop tracking double disposal in TaskThreadPool --- PerfectNumbers.Core/TaskThreadPool.cs | 179 ++++++++++++++++++++++---- 1 file changed, 153 insertions(+), 26 deletions(-) diff --git a/PerfectNumbers.Core/TaskThreadPool.cs b/PerfectNumbers.Core/TaskThreadPool.cs index a226c23..6dd7079 100644 --- a/PerfectNumbers.Core/TaskThreadPool.cs +++ b/PerfectNumbers.Core/TaskThreadPool.cs @@ -19,9 +19,16 @@ internal sealed class TaskThreadPool } private readonly ConcurrentQueue _pendingTaskQueue = new(); - private readonly BlockingCollection _pendingTasks; - private readonly Thread[] _threads; + private readonly HashSet _runningThreads = new(); + private readonly object _lock = new(); private readonly Action _taskExecutor; + private readonly int _maximumConcurrency; + private readonly SemaphoreSlim _workAvailable = new(0); + private readonly CancellationTokenSource _disposeCancellation = new(); + private int _pendingTaskCount; + private int _currentThreads; + private int _threadId; + public TaskThreadPool(int minimumThreads, Action taskExecutor) { if (minimumThreads <= 0) @@ -30,23 +37,28 @@ internal sealed class TaskThreadPool } _taskExecutor = taskExecutor ?? throw new ArgumentNullException(nameof(taskExecutor)); - _pendingTasks = new BlockingCollection(_pendingTaskQueue); - _threads = new Thread[minimumThreads]; + _maximumConcurrency = minimumThreads; - for (int i = 0; i < minimumThreads; i++) + List threadsToStart = new(minimumThreads); + + lock (_lock) { - Thread thread = new(WorkerLoop) + for (int i = 0; i < minimumThreads; i++) { - IsBackground = true, - Name = $"{nameof(TaskThreadPool)}-{i + 1}" - }; + Thread thread = CreateWorkerThread(); + _runningThreads.Add(thread); + _currentThreads++; + threadsToStart.Add(thread); + } + } - _threads[i] = thread; + foreach (Thread thread in threadsToStart) + { thread.Start(); } } - public int ThreadCount => _threads.Length; + public int ThreadCount => _maximumConcurrency; public void Queue(Task task) { @@ -55,18 +67,29 @@ internal sealed class TaskThreadPool throw new ArgumentNullException(nameof(task)); } - try + List? threadsToStart; + + lock (_lock) { - _pendingTasks.Add(new WorkItem(task)); - } - catch (Exception ex) - { - if (ex is InvalidOperationException || ex is ObjectDisposedException) + if (_disposeCancellation.IsCancellationRequested) { - throw new ObjectDisposedException(nameof(TaskThreadPool), ex); + throw new ObjectDisposedException(nameof(TaskThreadPool)); } - throw; + _pendingTaskQueue.Enqueue(new WorkItem(task)); + _pendingTaskCount++; + + threadsToStart = EnsureWorkerCapacityLocked(null); + } + + _workAvailable.Release(); + + if (threadsToStart is not null) + { + foreach (Thread thread in threadsToStart) + { + thread.Start(); + } } } @@ -91,28 +114,132 @@ internal sealed class TaskThreadPool public void Dispose() { - _pendingTasks.CompleteAdding(); + Thread[] threadsToJoin; - foreach (Thread thread in _threads) + lock (_lock) + { + _disposeCancellation.Cancel(); + + threadsToJoin = new Thread[_runningThreads.Count]; + _runningThreads.CopyTo(threadsToJoin); + } + + if (threadsToJoin.Length > 0) + { + _workAvailable.Release(threadsToJoin.Length); + } + + foreach (Thread thread in threadsToJoin) { thread.Join(); } - _pendingTasks.Dispose(); + _workAvailable.Dispose(); + _disposeCancellation.Dispose(); + + lock (_lock) + { + _runningThreads.Clear(); + + while (_pendingTaskQueue.TryDequeue(out _)) + { + } + + _pendingTaskCount = 0; + _currentThreads = 0; + } + } + + private bool TryDequeueWork(out WorkItem workItem) + { + CancellationToken cancellationToken = _disposeCancellation.Token; + + while (true) + { + try + { + _workAvailable.Wait(cancellationToken); + } + catch (OperationCanceledException) + { + workItem = default; + return false; + } + + if (_pendingTaskQueue.TryDequeue(out workItem)) + { + Interlocked.Decrement(ref _pendingTaskCount); + return true; + } + } } private void WorkerLoop() { try { - foreach (WorkItem workItem in _pendingTasks.GetConsumingEnumerable()) + if (!TryDequeueWork(out WorkItem workItem)) { - _taskExecutor(workItem.Task); + return; } + + _taskExecutor(workItem.Task); } - catch (ObjectDisposedException) + finally { - // The collection was disposed while the worker was waiting. Exit gracefully. + CompleteWorker(); } } + + private void CompleteWorker() + { + List? threadsToStart = null; + + lock (_lock) + { + _runningThreads.Remove(Thread.CurrentThread); + _currentThreads--; + + if (!_disposeCancellation.IsCancellationRequested) + { + threadsToStart = EnsureWorkerCapacityLocked(null); + } + } + + if (threadsToStart is not null) + { + foreach (Thread thread in threadsToStart) + { + thread.Start(); + } + } + } + + private List? EnsureWorkerCapacityLocked(List? threadsToStart) + { + int pendingTasks = Math.Min(_maximumConcurrency, Volatile.Read(ref _pendingTaskCount)); + + while (!_disposeCancellation.IsCancellationRequested && _currentThreads < pendingTasks) + { + Thread thread = CreateWorkerThread(); + _runningThreads.Add(thread); + _currentThreads++; + + threadsToStart ??= new List(); + threadsToStart.Add(thread); + } + + return threadsToStart; + } + + private Thread CreateWorkerThread() + { + int threadIndex = Interlocked.Increment(ref _threadId); + + return new Thread(WorkerLoop) + { + IsBackground = true, + Name = $"{nameof(TaskThreadPool)}-{threadIndex}" + }; + } }