fix(locking): release troubleshooting + playout locks on all terminal paths (#233, #234)
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 5s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 5m55s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 8m24s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 5s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 5m55s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 8m24s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Consume the EntityLocker ownership contract (#231/#241: Lock* returns true iff this caller won the slot) at three lock-leak sites surfaced by adversarial-reviewer#20. #233 (F3) — troubleshooting playback: - PrepareTroubleshootingPlaybackHandler: both lock sites now acquire via `if (!LockTroubleshootingPlayback())` (kills the check-then-set TOCTOU) and the empty-media-path Left return releases the lock it acquired — previously it leaked, wedging the status endpoint at "running" forever for a file gone from disk. - TroubleshootController.TroubleshootPlayback: lock conflict is now 409 ProblemDetails (was a bare 404, indistinguishable from a bad id); the Prepare-success -> enqueue window releases the lock if we never hand off to StartTroubleshootingPlayback. #234 (F4 + F5.2) — playout builds: - ExtractEmbeddedSubtitlesHandler: try/finally releases exactly the playouts it locked, on every terminal path (cancellation early-return, swallowed cancellation, any exception) — no more permanent leaks after cancelled mid-extraction, and no cross-release of playouts held by someone else. - BuildPlayoutHandler: skips (logs, returns Right) when LockPlayout returns false instead of building unlocked and cross-releasing the other owner's lock in finally. Tests: handler-level release-discipline tests (Prepare empty-path, Extract cancellation + no-cross-release, BuildPlayout skip + finally-release) via the InMemoryTvContext harness, a TroubleshootController 409 test, and OpenApi contract cases for the m3u8 endpoint's 409. OpenAPI regenerated. All non-vacuous (F3 verified against a negative control). Fixes #233, #234 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -114,6 +114,7 @@ public class TroubleshootController(
|
||||
[Tags("Troubleshooting")]
|
||||
[EndpointSummary("Start a troubleshooting playback session")]
|
||||
[EndpointGroupName("general")]
|
||||
[ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status409Conflict)]
|
||||
public async Task<IActionResult> TroubleshootPlayback(
|
||||
[FromQuery]
|
||||
int mediaItem,
|
||||
@@ -140,8 +141,22 @@ public class TroubleshootController(
|
||||
var sessionId = Guid.NewGuid();
|
||||
using var logContext = LogContext.PushProperty(InMemoryLogService.CorrelationIdKey, sessionId);
|
||||
|
||||
// acquiredLock: the handler took the troubleshooting lock on a successful Prepare.
|
||||
// startEnqueued: we handed off to StartTroubleshootingPlayback, which becomes the lock's releaser.
|
||||
var acquiredLock = false;
|
||||
var startEnqueued = false;
|
||||
|
||||
try
|
||||
{
|
||||
// fast lock-conflict signal so the client gets a 409 (not a 404) while another
|
||||
// troubleshooting session is active; the atomic acquire in the handler is the real guard
|
||||
if (entityLocker.IsTroubleshootingPlaybackLocked())
|
||||
{
|
||||
return ApiResults.ConflictProblem(
|
||||
"Troubleshooting playback in progress",
|
||||
"Another troubleshooting playback session is currently running. Try again once it completes.");
|
||||
}
|
||||
|
||||
Option<int> ss = seekSeconds > 0 ? seekSeconds : Option<int>.None;
|
||||
|
||||
Either<BaseError, PlayoutItemResult> result = await mediator.Send(
|
||||
@@ -164,6 +179,9 @@ public class TroubleshootController(
|
||||
return NotFound();
|
||||
}
|
||||
|
||||
// Prepare returned a process, so the handler holds the troubleshooting lock now
|
||||
acquiredLock = true;
|
||||
|
||||
foreach (PlayoutItemResult playoutItemResult in result.RightToSeq())
|
||||
{
|
||||
Either<BaseError, MediaItemInfo> maybeMediaInfo =
|
||||
@@ -192,6 +210,9 @@ public class TroubleshootController(
|
||||
troubleshootingInfo),
|
||||
cancellationToken);
|
||||
|
||||
// StartTroubleshootingPlayback is now responsible for releasing the lock in its finally
|
||||
startEnqueued = true;
|
||||
|
||||
string playlistFile = Path.Combine(FileSystemLayout.TranscodeTroubleshootingFolder, "live.m3u8");
|
||||
while (!fileSystem.File.Exists(playlistFile))
|
||||
{
|
||||
@@ -242,6 +263,15 @@ public class TroubleshootController(
|
||||
{
|
||||
// do nothing
|
||||
}
|
||||
finally
|
||||
{
|
||||
// the handler acquired the lock but we never handed off to the worker
|
||||
// (cancellation/exception in the window before enqueue) -> release it so it doesn't leak
|
||||
if (acquiredLock && !startEnqueued)
|
||||
{
|
||||
entityLocker.UnlockTroubleshootingPlayback();
|
||||
}
|
||||
}
|
||||
|
||||
return NotFound();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user