Skip to content

lifecycle-poc: close(registered) releases workers before runningPipelines.Set, so a concurrent Stop can hit the dead run #2833

Description

@devarispbrown

Summary

pkg/lifecycle-poc/service.go's runPipeline orders:

close(registered)  ->  runningPipelines.Set(...)  ->  UpdateStatus(StatusRunning)

Between the first two statements, worker goroutines are already released and moving records, while
runningPipelines[id] still points at the dead run on a recovery restart. A Stop landing in
that window stops the dead run and the live one keeps going.

This is #2746's failure mode with a much smaller window — two adjacent statements, no I/O between
them. #2812 narrowed it rather than closing it.

Why it matters

Everything public resolves a pipeline through runningPipelines: Stop, StopAll,
WaitPipeline, StopAndWait, and therefore provisioning.ApplyPlanLive. A caller that observes
the pipeline as running and issues a Stop can have that Stop applied to a run that has already
exited, leaving the live run unstopped. Data-integrity invariant 7 (shutdown is graceful by
default) is the one at risk.

pkg/lifecycle-poc becomes the default engine in v0.21, which raises the value of closing this
properly rather than narrowing it again.

Proposed fix

Move close(registered) to after the runningPipelines.Set(...). This is free and consistent
with that code's own stated constraint that it must happen before the UpdateStatus call — the
constraint is about ordering against the status write, not against the map write.

Why this isn't already a PR

Found while root-causing the flake fixed in #2832, which was test-only. This one is Tier 1
(production shutdown/recovery ordering), and a deterministic regression test needs a way to hold the
window open between two adjacent statements — which means a production test seam. CLAUDE.md forbids
shipping a fix without the test that would have caught it, so the seam design is the real work here
and belongs in its own PR with a failure-mode analysis.

Acceptance criteria

  • close(registered) happens after runningPipelines.Set(...)
  • A regression test holds the window open and asserts a concurrent Stop reaches the live
    run, not the dead one — failing without the reorder, passing with it
  • Failure-mode analysis in the PR body (Tier 1)
  • tests/chaos (race, x3) green

Related: #2746, #2806, #2812, #2832

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions