fix(536): clamp an unbalanced work-ahead release instead of going negative
Build ErsatzTV Image / CI image pin matches docker/ci (pull_request) Successful in 13s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 14s
Build ErsatzTV Image / decisions lifecycle (pull_request) Successful in 14s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 18s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 17s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m17s
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 15m10s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 19m37s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / CI image pin matches docker/ci (pull_request) Successful in 13s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 14s
Build ErsatzTV Image / decisions lifecycle (pull_request) Successful in 14s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 18s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 17s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 6m17s
Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 15m10s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 19m37s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Pre-push adversarial review, Low finding. The pool is process-wide and lives for the life of the app, so a `Release()` not matched by a successful `TryAcquire` would drive the count to -1 permanently: with a limit of 1 that silently admits two unthrottled transcodes forever, which is exactly the #529 QSV pool exhaustion with nothing in the logs to find it by. Clamp at zero and record the breakage in `UnbalancedReleases` rather than throwing — the sole caller releases from a `finally`, where a throw would swallow the real exception. The hammer tests now also assert `UnbalancedReleases == 0`, so the clamp cannot mask drift it was added to survive. Also moves the #536 index line to the end of the in-file decisions index (it was inserted in the 2026-07-11 block while its body appends at the end) — review nit, anchors were already correct. refs #536
This commit is contained in:
@@ -20,6 +20,7 @@ namespace ErsatzTV.Application.Streaming;
|
||||
public sealed class WorkAheadSlots
|
||||
{
|
||||
private int _count;
|
||||
private int _unbalancedReleases;
|
||||
|
||||
/// <summary>
|
||||
/// Gets the number of slots currently held. For diagnostics and tests only — never branch on
|
||||
@@ -51,8 +52,40 @@ public sealed class WorkAheadSlots
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Gets the number of releases that were not matched by a successful acquire. Always zero in a
|
||||
/// correct program; a non-zero value means the ownership contract was broken somewhere.
|
||||
/// </summary>
|
||||
public int UnbalancedReleases => Volatile.Read(ref _unbalancedReleases);
|
||||
|
||||
/// <summary>
|
||||
/// Returns a slot claimed by <see cref="TryAcquire" />. Only ever called by the owner of that slot.
|
||||
/// </summary>
|
||||
public void Release() => Interlocked.Decrement(ref _count);
|
||||
/// <remarks>
|
||||
/// Ownership is a discipline, not a token — the same call-once contract as `EntityLocker` (#231).
|
||||
/// The one failure this does defend against is an unbalanced release driving the count negative:
|
||||
/// this pool is process-wide and lives for the life of the app, so a count of -1 would silently
|
||||
/// and permanently admit one extra unthrottled transcode, re-opening the #529 QSV pool
|
||||
/// exhaustion with nothing in the logs to find it by. Clamping keeps the resource guarantee
|
||||
/// intact and records the breakage in <see cref="UnbalancedReleases" /> instead of throwing —
|
||||
/// the single caller releases from a `finally`, where a throw would swallow the real exception.
|
||||
/// </remarks>
|
||||
public void Release()
|
||||
{
|
||||
if (Interlocked.Decrement(ref _count) >= 0)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
Interlocked.Increment(ref _unbalancedReleases);
|
||||
|
||||
while (true)
|
||||
{
|
||||
int current = Volatile.Read(ref _count);
|
||||
if (current >= 0 || Interlocked.CompareExchange(ref _count, 0, current) == current)
|
||||
{
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -59,6 +59,34 @@ public class WorkAheadSlotsTests
|
||||
slots.Count.ShouldBe(0);
|
||||
}
|
||||
|
||||
// The pool is process-wide and never recreated, so a negative count would not self-heal: it would
|
||||
// permanently admit more than `limit` unthrottled transcodes, with nothing in the logs to find it by.
|
||||
[Test]
|
||||
public void Release_WithoutAcquire_ClampsAtZeroAndIsRecorded()
|
||||
{
|
||||
var slots = new WorkAheadSlots();
|
||||
|
||||
slots.Release();
|
||||
|
||||
slots.Count.ShouldBe(0);
|
||||
slots.UnbalancedReleases.ShouldBe(1);
|
||||
|
||||
// the budget is intact: a limit of 1 still admits exactly one holder, not two
|
||||
slots.TryAcquire(1).ShouldBeTrue();
|
||||
slots.TryAcquire(1).ShouldBeFalse();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void UnbalancedReleases_IsZeroUnderCorrectUse()
|
||||
{
|
||||
var slots = new WorkAheadSlots();
|
||||
|
||||
slots.TryAcquire(1).ShouldBeTrue();
|
||||
slots.Release();
|
||||
|
||||
slots.UnbalancedReleases.ShouldBe(0);
|
||||
}
|
||||
|
||||
// Hammers the acquire race over many rounds rather than a single simultaneous burst: the
|
||||
// read->increment window is far too narrow to collide reliably when threads release once.
|
||||
[Test]
|
||||
@@ -120,6 +148,7 @@ public class WorkAheadSlotsTests
|
||||
|
||||
badRounds.ShouldBe(0);
|
||||
slots.Count.ShouldBe(0);
|
||||
slots.UnbalancedReleases.ShouldBe(0);
|
||||
}
|
||||
|
||||
// The release path is the fiddly half: a slot freed by its owner must become available again,
|
||||
@@ -166,5 +195,6 @@ public class WorkAheadSlotsTests
|
||||
|
||||
overLimit.ShouldBe(0);
|
||||
slots.Count.ShouldBe(0);
|
||||
slots.UnbalancedReleases.ShouldBe(0);
|
||||
}
|
||||
}
|
||||
|
||||
+1
-1
@@ -92,7 +92,6 @@ in-file entries.
|
||||
- [2026-07-11 — Schedules SPA editor: draft/explicit-Save over instant-persist; Copy includes multi/smart/rerun; shuffled-GET normalization preserved](#2026-07-11--schedules-spa-editor-draftexplicit-save-over-instant-persist-copy-includes-multismartrerun-shuffled-get-normalization-preserved)
|
||||
- [2026-07-11 — Channel editor: bare-create entry point + external-logo mutual exclusion (#212)](#2026-07-11--channel-editor-bare-create-entry-point--external-logo-mutual-exclusion-212)
|
||||
- [2026-07-11 — EntityLocker: atomic flags + single-owner release discipline, no owner tokens (#231)](#2026-07-11--entitylocker-atomic-flags--single-owner-release-discipline-no-owner-tokens-231)
|
||||
- [2026-07-21 — Work-ahead slots are claimed atomically by the caller, released by the transcode it hands them to (#536)](#2026-07-21--work-ahead-slots-are-claimed-atomically-by-the-caller-released-by-the-transcode-it-hands-them-to-536)
|
||||
- [2026-07-11 — Media-source management REST write API + SPA (#202)](#2026-07-11--media-source-management-rest-write-api--spa-202)
|
||||
- [2026-07-11 — Legacy→SPA redirect matcher: exact map + ordered segment-template patterns (#204)](#2026-07-11--legacyspa-redirect-matcher-exact-map--ordered-segment-template-patterns-204)
|
||||
- [2026-07-11 — Pre-removal Blazor rollback tag `blazor-final` (#205)](#2026-07-11--pre-removal-blazor-rollback-tag-blazor-final-205)
|
||||
@@ -133,6 +132,7 @@ in-file entries.
|
||||
- [2026-07-21 — Parallel orientation + selection is the startup protocol; #237 retired (#520)](#2026-07-21--parallel-orientation--selection-is-the-startup-protocol-237-retired-520)
|
||||
- [2026-07-21 — External channel-logo URLs are downloaded and cached at save time; the render path never fetches a logo (#525)](#2026-07-21--external-channel-logo-urls-are-downloaded-and-cached-at-save-time-the-render-path-never-fetches-a-logo-525)
|
||||
- [2026-07-21 — Check the worked issue before the decision corpus; a closed tracker's comments need no retrofit (#524)](#2026-07-21--check-the-worked-issue-before-the-decision-corpus-a-closed-trackers-comments-need-no-retrofit-524)
|
||||
- [2026-07-21 — Work-ahead slots are claimed atomically by the caller, released by the transcode it hands them to (#536)](#2026-07-21--work-ahead-slots-are-claimed-atomically-by-the-caller-released-by-the-transcode-it-hands-them-to-536)
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user