fix(site-runtime): reuse exited detached worker slots in script scheduler (review 2026-10-21 01/SR-M1)

This commit is contained in:
Joseph Doherty
2026-10-22 08:29:09 -04:00
parent 96cab3f3d7
commit 7541a38762
2 changed files with 91 additions and 11 deletions
@@ -307,17 +307,43 @@ public sealed class ScriptExecutionScheduler : TaskScheduler, IDisposable
} }
} }
/// <summary>Appends <paramref name="count"/> new worker slots + threads. Caller holds <see cref="_growLock"/>.</summary> /// <summary>
/// Adds <paramref name="count"/> workers. Slots vacated by an exited detached worker are
/// reused first (review 2026-10-21 01/SR-M1: otherwise a script that wedges on every run
/// grows <see cref="_slots"/> and starts a fresh slot per detach for the process lifetime);
/// only the remainder appends new slots. Caller holds <see cref="_growLock"/>.
/// </summary>
private void GrowTo(int count) private void GrowTo(int count)
{ {
var existing = _slots; var existing = _slots;
var grown = new WorkerSlot[existing.Length + count]; var reuse = new List<int>();
Array.Copy(existing, grown, existing.Length); for (var i = 0; i < existing.Length && reuse.Count < count; i++)
if (Volatile.Read(ref existing[i].Free) != 0) reuse.Add(i);
for (var i = 0; i < count; i++) var appended = count - reuse.Count;
var grown = existing;
if (appended > 0)
{ {
var index = existing.Length + i; grown = new WorkerSlot[existing.Length + appended];
grown[index] = new WorkerSlot(); Array.Copy(existing, grown, existing.Length);
for (var i = 0; i < appended; i++)
grown[existing.Length + i] = new WorkerSlot();
}
var indices = new List<int>(count);
indices.AddRange(reuse);
for (var i = 0; i < appended; i++)
indices.Add(existing.Length + i);
// Reset reused slots BEFORE the thread starts; the exited worker never touches its slot
// again after marking it Free.
foreach (var index in reuse)
{
var slot = grown[index];
Volatile.Write(ref slot.Detached, 0);
Volatile.Write(ref slot.RunStamp, 0);
Volatile.Write(ref slot.BusySinceTicks, 0);
Volatile.Write(ref slot.Free, 0);
} }
// Publish the array BEFORE starting the threads: a worker's very first action is to // Publish the array BEFORE starting the threads: a worker's very first action is to
@@ -325,19 +351,22 @@ public sealed class ScriptExecutionScheduler : TaskScheduler, IDisposable
_slots = grown; _slots = grown;
_configuredCount += count; _configuredCount += count;
for (var i = 0; i < count; i++) foreach (var index in indices)
{ {
var index = existing.Length + i; var i = index;
var thread = new Thread(() => WorkerLoop(index)) var thread = new Thread(() => WorkerLoop(i))
{ {
IsBackground = true, IsBackground = true,
Name = ThreadNamePrefix + index Name = ThreadNamePrefix + i
}; };
grown[index].Thread = thread; grown[i].Thread = thread;
thread.Start(); thread.Start();
} }
} }
/// <summary>Total worker slots (live + detached-but-unexited + free). Diagnostic/test seam.</summary>
internal int SlotCount => _slots.Length;
[ThreadStatic] [ThreadStatic]
private static ScriptExecutionScheduler? CurrentScheduler; private static ScriptExecutionScheduler? CurrentScheduler;
@@ -368,6 +397,14 @@ public sealed class ScriptExecutionScheduler : TaskScheduler, IDisposable
if (Volatile.Read(ref slot.Detached) != 0) if (Volatile.Read(ref slot.Detached) != 0)
{ {
Interlocked.Decrement(ref _detachedLive); Interlocked.Decrement(ref _detachedLive);
// Hand the slot back for reuse by the next GrowTo. Under the lock so a
// concurrent GrowTo sees either "not free" or a fully vacated slot; the
// thread touches no slot state after this.
lock (_growLock)
{
slot.Thread = null;
Volatile.Write(ref slot.Free, 1);
}
return; return;
} }
} }
@@ -435,6 +472,9 @@ public sealed class ScriptExecutionScheduler : TaskScheduler, IDisposable
/// <summary>Non-zero once the worker has been detached; it exits after its current task.</summary> /// <summary>Non-zero once the worker has been detached; it exits after its current task.</summary>
public int Detached; public int Detached;
/// <summary>Non-zero once a detached worker has exited and the slot may be reused by <see cref="GrowTo"/>.</summary>
public int Free;
/// <summary>The worker thread, for <see cref="Dispose"/> to join.</summary> /// <summary>The worker thread, for <see cref="Dispose"/> to join.</summary>
public Thread? Thread; public Thread? Thread;
} }
@@ -118,4 +118,44 @@ public class ScriptExecutionSchedulerTests
await Task.Delay(50); await Task.Delay(50);
Assert.True(condition(), "condition not met within timeout"); Assert.True(condition(), "condition not met within timeout");
} }
/// <summary>
/// Repeatedly wedging + detaching a worker must reuse the vacated slot rather than grow the
/// slot table without bound (review 2026-10-21 01/SR-M1).
/// </summary>
[Fact]
public async Task DetachedWorkerSlot_IsReused_SlotTableStaysBounded()
{
using var scheduler = new ScriptExecutionScheduler(1);
for (var round = 0; round < 6; round++)
{
var gate = new ManualResetEventSlim(false);
var started = new TaskCompletionSource<(int Slot, long Stamp)>();
var wedged = Task.Factory.StartNew(() =>
{
var slot = ScriptExecutionScheduler.CurrentWorkerSlot!.Value;
started.SetResult((slot, scheduler.CurrentRunStamp(slot)));
gate.Wait();
}, CancellationToken.None, TaskCreationOptions.None, scheduler);
var (s, stamp) = await started.Task;
Assert.Equal(WorkerDetachOutcome.Detached, scheduler.TryDetachWorker(s, stamp));
gate.Set();
await wedged;
// Wait for the detached worker to exit and hand its slot back.
var deadline = DateTime.UtcNow.AddSeconds(5);
while (scheduler.DetachedThreadCount != 0 && DateTime.UtcNow < deadline)
await Task.Delay(10);
Assert.Equal(0, scheduler.DetachedThreadCount);
await Task.Delay(50);
}
// 1 live + at most 1 free/in-flight detached slot, never 1 + 6.
Assert.True(scheduler.SlotCount <= 2, $"slot table grew to {scheduler.SlotCount}");
// The pool still works afterwards.
var ran = await Task.Factory.StartNew(() => 42, CancellationToken.None, TaskCreationOptions.None, scheduler);
Assert.Equal(42, ran);
}
} }