Skip to content

Commit 5976abd

Browse files
paulirwinclaude
andauthored
Cancellation support for LimitedConcurrencyLevelTaskScheduler, #1253 (#1275)
* Cancellation support for LimitedConcurrencyLevelTaskScheduler, #1253 * Remove CancelAsync that is not available on .NET Framework * Throw on QueueTask after shutdown in LimitedConcurrencyLevelTaskScheduler, #1253 Match Java's ExecutorService.shutdown() RejectedExecutionException semantics instead of silently dropping post-shutdown submissions, which would leave awaiters hanging on a Task that never completes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Optimize cleanup of cancellation token source --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5381fdb commit 5976abd

6 files changed

Lines changed: 125 additions & 46 deletions

File tree

src/Lucene.Net.TestFramework/Support/Threading/LimitedConcurrencyLevelTaskScheduler.cs

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ to the software or derivative works that you create that run directly on a Micro
4848
Office or Microsoft Dynamics).
4949
*/
5050

51-
using J2N.Threading.Atomic;
5251
using System;
5352
using System.Collections.Generic;
5453
using System.Threading;
@@ -64,8 +63,6 @@ namespace Lucene.Net.Support.Threading
6463
/// </summary>
6564
internal class LimitedConcurrencyLevelTaskScheduler : TaskScheduler
6665
{
67-
private readonly AtomicBoolean shutDown = new AtomicBoolean(false);
68-
6966
// Indicates whether the current thread is processing work items.
7067
[ThreadStatic]
7168
private static bool _currentThreadIsProcessingItems;
@@ -76,21 +73,42 @@ internal class LimitedConcurrencyLevelTaskScheduler : TaskScheduler
7673
// The maximum concurrency level allowed by this scheduler.
7774
private readonly int _maxDegreeOfParallelism;
7875

76+
// A cancellation token for preventing queueing new work when shut down
77+
private readonly CancellationToken _cancellationToken;
78+
7979
// Indicates whether the scheduler is currently processing work items.
8080
private int _delegatesQueuedOrRunning = 0;
8181

82-
// Creates a new instance with the specified degree of parallelism.
83-
public LimitedConcurrencyLevelTaskScheduler(int maxDegreeOfParallelism)
82+
/// <summary>
83+
/// Creates a new instance with the specified degree of parallelism.
84+
/// </summary>
85+
/// <param name="maxDegreeOfParallelism">The max degree of parallelism for tasks.</param>
86+
/// <param name="cancellationToken">
87+
/// A cancellation token that is used to shut down the task scheduler in an orderly manner.
88+
/// If cancellation is requested, this scheduler will throw a <see cref="TaskSchedulerException"/> if a
89+
/// new task is attempted to be queued.
90+
/// This behaves like <c>ExecutorService.shutdown()</c> in Java (with <c>RejectedExecutionException</c>),
91+
/// allowing any running tasks to finish.
92+
/// </param>
93+
/// <exception cref="ArgumentOutOfRangeException">if <paramref name="maxDegreeOfParallelism"/> is less than 1.</exception>
94+
public LimitedConcurrencyLevelTaskScheduler(int maxDegreeOfParallelism, CancellationToken cancellationToken = default)
8495
{
8596
if (maxDegreeOfParallelism < 1) throw new ArgumentOutOfRangeException(nameof(maxDegreeOfParallelism));
8697
_maxDegreeOfParallelism = maxDegreeOfParallelism;
98+
_cancellationToken = cancellationToken;
8799
}
88100

89101
// Queues a task to the scheduler.
90102
protected sealed override void QueueTask(Task task)
91103
{
92104
// Don't queue any more work.
93-
if (shutDown) return;
105+
if (_cancellationToken.IsCancellationRequested)
106+
{
107+
// LUCENENET NOTE: in Java's `ExecutorService.shutdown()`, this would be RejectedExecutionException.
108+
// The equivalent for a TaskScheduler is TaskSchedulerException, but the framework will wrap what
109+
// we throw here in a TaskSchedulerException.
110+
throw new InvalidOperationException("The task scheduler was shut down and is not accepting new tasks.");
111+
}
94112

95113
// Add the task to the list of tasks to be processed. If there aren't enough
96114
// delegates currently queued or running to process tasks, schedule another.
@@ -203,17 +221,13 @@ protected sealed override IEnumerable<Task> GetScheduledTasks()
203221
}
204222
}
205223

206-
/// <summary>
207-
/// Stops this TaskScheduler from queuing new tasks.
208-
/// </summary>
209-
public void Shutdown()
210-
{
211-
shutDown.Value = true;
212-
}
213-
214224
/// <summary>
215225
/// Gets a value indicating whether this TaskScheduler has been shut down.
216226
/// </summary>
217-
public bool IsShutdown => shutDown;
227+
/// <remarks>
228+
/// This simply returns whether the cancellation token provided to the constructor
229+
/// has requested cancellation.
230+
/// </remarks>
231+
public bool IsShutdown => _cancellationToken.IsCancellationRequested;
218232
}
219233
}

src/Lucene.Net.TestFramework/Util/LuceneTestCase.cs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2084,14 +2084,16 @@ public static IndexSearcher NewSearcher(IndexReader r, bool maybeWrap, bool wrap
20842084
{
20852085
int threads = 0;
20862086
LimitedConcurrencyLevelTaskScheduler ex;
2087+
CancellationTokenSource cts = null;
20872088
if (random.NextBoolean())
20882089
{
20892090
ex = null;
20902091
}
20912092
else
20922093
{
2094+
cts = new CancellationTokenSource(); // LUCENENET NOTE: this is cleaned up in ReaderClosedListenerAnonymousClass
20932095
threads = TestUtil.NextInt32(random, 1, 8);
2094-
ex = new LimitedConcurrencyLevelTaskScheduler(threads);
2096+
ex = new LimitedConcurrencyLevelTaskScheduler(threads, cts.Token);
20952097
//ex = new ThreadPoolExecutor(threads, threads, 0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<IThreadRunnable>(), new NamedThreadFactory("LuceneTestCase"));
20962098
// uncomment to intensify LUCENE-3840
20972099
// ex.prestartAllCoreThreads();
@@ -2102,7 +2104,7 @@ public static IndexSearcher NewSearcher(IndexReader r, bool maybeWrap, bool wrap
21022104
{
21032105
Console.WriteLine("NOTE: newSearcher using ExecutorService with " + threads + " threads");
21042106
}
2105-
r.AddReaderDisposedListener(new ReaderClosedListenerAnonymousClass(ex));
2107+
r.AddReaderDisposedListener(new ReaderClosedListenerAnonymousClass(cts));
21062108
}
21072109
IndexSearcher ret;
21082110
if (wrapWithAssertions)
@@ -3262,20 +3264,25 @@ public static double RandomGaussian() // LUCENENET: CA1822: Mark members as stat
32623264
return Random.NextGaussian();
32633265
}
32643266

3267+
#nullable enable
32653268
private sealed class ReaderClosedListenerAnonymousClass : IReaderDisposedListener
32663269
{
3267-
private readonly LimitedConcurrencyLevelTaskScheduler ex;
3270+
private readonly CancellationTokenSource? cts; // LUCENENET-specific: cancellation support, can be null
32683271

3269-
public ReaderClosedListenerAnonymousClass(LimitedConcurrencyLevelTaskScheduler ex)
3272+
public ReaderClosedListenerAnonymousClass(CancellationTokenSource? cts)
32703273
{
3271-
this.ex = ex;
3274+
this.cts = cts;
32723275
}
32733276

32743277
public void OnDispose(IndexReader reader)
32753278
{
3276-
ex?.Shutdown();
3277-
//TestUtil.ShutdownExecutorService(ex);
3279+
if (cts != null)
3280+
{
3281+
cts.Cancel();
3282+
cts.Dispose();
3283+
}
32783284
}
32793285
}
3286+
#nullable restore
32803287
}
32813288
}

src/Lucene.Net.Tests.Suggest/Spell/TestSpellChecker.cs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -472,11 +472,19 @@ public async Task TestConcurrentAccess()
472472
int numThreads = 5 + Random.nextInt(5);
473473
var tasks = new ConcurrentBag<Task>();
474474
SpellCheckWorker[] workers = new SpellCheckWorker[numThreads];
475-
var executor = new LimitedConcurrencyLevelTaskScheduler(numThreads); // LUCENENET NOTE: Not sure why in Java they decided to pass the max concurrent threads as all of the threads, but this demonstrates how to use a custom TaskScheduler in .NET.
475+
476+
// LUCENENET NOTE: Not sure why in Java they decided to pass the max concurrent threads as all of the threads, but this demonstrates how to use a custom TaskScheduler in .NET.
477+
// LUCENENET NOTE: This cancellation token/source is intentionally separate from the one below, because it is solely used as an equivalent
478+
// to ExecutorService.shutdown(), which just stops queueing new tasks.
479+
using var executorShutdown = new CancellationTokenSource();
480+
var executor = new LimitedConcurrencyLevelTaskScheduler(numThreads, executorShutdown.Token);
481+
476482
using var shutdown = new CancellationTokenSource();
477483
var cancellationToken = shutdown.Token;
484+
478485
var stop = new AtomicBoolean(false);
479486
var taskFactory = new TaskFactory(executor);
487+
480488
for (int i = 0; i < numThreads; i++)
481489
{
482490
SpellCheckWorker spellCheckWorker = new SpellCheckWorker(this, r, stop, cancellationToken, taskNum: i);
@@ -495,7 +503,12 @@ public async Task TestConcurrentAccess()
495503
}
496504

497505
stop.Value = true;
498-
executor.Shutdown(); // Stop allowing tasks to queue
506+
507+
// LUCENENET NOTE: This is technically pointless, since by this point we've already queued all the tasks.
508+
// Leaving this here for compatibility and possible future test changes that might use it properly.
509+
// ReSharper disable once MethodHasAsyncOverload - not available in .NET Framework
510+
executorShutdown.Cancel(); // Stop queueing new tasks
511+
499512
try
500513
{
501514
// wait for 60 seconds - usually this is very fast but coverage runs could take quite long
@@ -526,7 +539,7 @@ public async Task TestConcurrentAccess()
526539
AssertSearchersClosed();
527540
}
528541

529-
private void AssertLastSearcherOpen(int numSearchers)
542+
private static void AssertLastSearcherOpen(int numSearchers)
530543
{
531544
assertEquals(numSearchers, searchers.Count);
532545
IndexSearcher[] searcherArray = searchers.ToArray();
@@ -545,7 +558,7 @@ private void AssertLastSearcherOpen(int numSearchers)
545558
}
546559
}
547560

548-
private void AssertSearchersClosed()
561+
private static void AssertSearchersClosed()
549562
{
550563
foreach (IndexSearcher searcher in searchers)
551564
{

src/Lucene.Net.Tests/Search/TestIndexSearcher.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ public virtual void TestHugeN()
134134
}
135135
}
136136

137-
// LUCENENET: .NET doesn't have a way to shut down the TaskScheduler explicitly
137+
// LUCENENET: shutdown not needed here since all searches above run synchronously
138138
//TestUtil.ShutdownExecutorService(service);
139139
}
140140

@@ -361,7 +361,7 @@ public virtual void TestCancellation_MultiThreaded_CancelDuringSearch_Throws()
361361
Assume.That(r.Leaves.Count >= 2, "Test requires a multi-segment index");
362362

363363
using CancellationTokenSource cts = new CancellationTokenSource();
364-
TaskScheduler service = new LimitedConcurrencyLevelTaskScheduler(4);
364+
TaskScheduler service = new LimitedConcurrencyLevelTaskScheduler(4); // LUCENENET NOTE: intentionally NOT passing cts.Token here since that parameter is for shutdown only, and that's not what we're testing
365365
IndexSearcher searcher = new CancelAfterFirstLeafSearcher(r, service, cts);
366366

367367
Exception ex = Assert.Catch(() => searcher.Search(new MatchAllDocsQuery(), 10, cts.Token));

src/Lucene.Net.Tests/Support/Threading/JSR166TestCase.cs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,8 @@ public void joinPool(TaskScheduler exec)
351351
{
352352
try
353353
{
354-
exec.Shutdown();
354+
// LUCENENET NOTE: no need to call a shutdown cancellation token here,
355+
// since any tasks would already be queued by the time we get here
355356
assertTrue(exec.AwaitTermination(TimeSpan.FromMilliseconds(LONG_DELAY_MS)));
356357
}
357358
// catch (SecurityException ok) // LUCENENET - not needed
@@ -448,6 +449,9 @@ public TaskState(TaskScheduler scheduler)
448449

449450
public void NewTask(Action action)
450451
{
452+
if (_factory.Scheduler is LimitedConcurrencyLevelTaskScheduler { IsShutdown: true })
453+
return;
454+
451455
var task = _factory.StartNew(action);
452456
_tasks.Add(task);
453457
}
@@ -517,14 +521,6 @@ public static int GetTaskCount(this TaskScheduler scheduler)
517521
return 0;
518522
}
519523

520-
public static void Shutdown(this TaskScheduler scheduler)
521-
{
522-
if (scheduler is LimitedConcurrencyLevelTaskScheduler lcl)
523-
{
524-
lcl.Shutdown();
525-
}
526-
}
527-
528524
public static bool IsTerminated(this TaskScheduler scheduler)
529525
{
530526
if (scheduler is LimitedConcurrencyLevelTaskScheduler lcl

src/Lucene.Net.Tests/Support/Threading/TestLimitedConcurrencyLevelTaskScheduler.cs

Lines changed: 58 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Based on tests from Apache Harmony:
22
// https://github.com/apache/harmony/blob/02970cb7227a335edd2c8457ebdde0195a735733/classlib/modules/concurrent/src/test/java/ThreadPoolExecutorTest.java
33

4+
using Lucene.Net.Attributes;
45
using NUnit.Framework;
56
using System;
67
using System.Threading;
@@ -135,7 +136,6 @@ public void TestGetCompletedTaskCount()
135136
AssumeTrue($"Expected 1, but got {p2.GetCompletedTaskCount()} - this may be a timing issue.", p2.GetCompletedTaskCount() == 1);
136137

137138
// LUCENENET NOTE: not catching SecurityException because that's not relevant here
138-
p2.Shutdown();
139139
joinPool(p2);
140140
}
141141

@@ -191,18 +191,64 @@ public void TestGetTaskCount()
191191
}
192192

193193
/// <summary>
194-
/// <see cref="LimitedConcurrencyLevelTaskScheduler.IsShutdown"/> is false before shutdown, true after
194+
/// Tests that a canceled token does not queue new tasks
195195
/// </summary>
196196
[Test]
197-
public void TestIsShutdown()
197+
public void TestCancellation()
198198
{
199-
var p1 = new LimitedConcurrencyLevelTaskScheduler(1);
200-
assertFalse(p1.IsShutdown);
201-
p1.Shutdown(); // LUCENENET NOTE: not catching SecurityException because that's not relevant here
202-
assertTrue(p1.IsShutdown);
199+
using var cts = new CancellationTokenSource();
200+
TaskScheduler p1 = new LimitedConcurrencyLevelTaskScheduler(1, cts.Token);
201+
202+
assertEquals(0, p1.GetTaskCount());
203+
204+
try
205+
{
206+
p1.Execute(MediumRunnable); // queue first task, not yet canceled
207+
// NOTE: no need to sleep here, we're just checking if tasks got queued
208+
assertEquals(1, p1.GetTaskCount());
209+
210+
cts.Cancel();
211+
p1.Execute(MediumRunnable); // queue second task (or try to)
212+
assertEquals(1, p1.GetTaskCount()); // cancellation should prevent queueing new tasks
213+
}
214+
catch (Exception)
215+
{
216+
unexpectedException();
217+
}
218+
219+
assertTrue(cts.Token.IsCancellationRequested);
203220
joinPool(p1);
204221
}
205222

223+
/// <summary>
224+
/// Submitting a task after shutdown via <see cref="TaskFactory.StartNew(Action)"/>
225+
/// surfaces an <see cref="InvalidOperationException"/> (wrapped by the TPL in a
226+
/// <see cref="TaskSchedulerException"/>), matching Java's
227+
/// <c>RejectedExecutionException</c> semantics for <c>ExecutorService</c>
228+
/// after <c>shutdown()</c>. Silently dropping the task would let awaiters
229+
/// hang on a Task that will never complete.
230+
/// </summary>
231+
/// <remarks>
232+
/// LUCENENET specific - this exercises the direct <see cref="TaskScheduler"/>
233+
/// submission path used outside of <see cref="JSR166TestCaseExtensions"/>
234+
/// (e.g., <see cref="Lucene.Net.Search.IndexSearcher"/>'s parallel slice path).
235+
/// </remarks>
236+
[Test, LuceneNetSpecific]
237+
public void TestQueueTaskAfterShutdownThrows()
238+
{
239+
using var cts = new CancellationTokenSource();
240+
var scheduler = new LimitedConcurrencyLevelTaskScheduler(1, cts.Token);
241+
var factory = new TaskFactory(scheduler);
242+
243+
cts.Cancel();
244+
245+
// NOTE: we intentionally do not want to pass cts.Token to StartNew, to test QueueTask.
246+
var ex = Assert.Throws<TaskSchedulerException>(() => factory.StartNew(() => { }, CancellationToken.None));
247+
assertTrue(
248+
$"Expected InvalidOperationException inner, got {ex!.InnerException?.GetType().FullName}",
249+
ex.InnerException is InvalidOperationException);
250+
}
251+
206252
/// <summary>
207253
/// isTerminated is false before termination, true after
208254
/// </summary>
@@ -215,7 +261,8 @@ public void TestIsShutdown()
215261
[Test]
216262
public void TestIsTerminated()
217263
{
218-
TaskScheduler p1 = new LimitedConcurrencyLevelTaskScheduler(1);
264+
using var cts = new CancellationTokenSource();
265+
TaskScheduler p1 = new LimitedConcurrencyLevelTaskScheduler(1, cts.Token);
219266
assertFalse(p1.IsTerminated());
220267

221268
try
@@ -224,7 +271,9 @@ public void TestIsTerminated()
224271
}
225272
finally
226273
{
227-
p1.Shutdown(); // LUCENENET NOTE: not catching SecurityException because that's not relevant here
274+
// LUCENENET NOTE: not catching SecurityException because that's not relevant here
275+
// LUCENENET NOTE: canceling here is of questionable utility, since no more tasks are queued
276+
cts.Cancel();
228277
}
229278

230279
try

0 commit comments

Comments
 (0)