Skip to content

WriteJson reads Elapsed() and Records() under TWO separate mutex acquisitions, so wall_seconds and the record set are no longer one snapshot #1720

Description

@localai-bot

PR #1711 moves PhaseLog::WriteJson's clock read ABOVE its ByStart(Records()), so that the writer's own copy and sort stop being charged to the render's wall and therefore to unaccounted_seconds. That is the defect #1569 tracks and the repair is right. It has a cost, and this issue is that cost recorded rather than hidden.

The two statements now take the process-wide mutex TWICE:

const double wall = Elapsed();                              // one acquisition
const std::vector<Record> records = ByStart(Records());     // another

so wall_seconds and the record set it is compared against come from two different instants. Under the previous order the pair was effectively one snapshot in the direction that matters — the clock was read last, so wall >= max(end_seconds) held by construction. It no longer does.

The observable if it ever broke is a NEGATIVE tail gap, which gaps reports and which ltx2 phase log: the emitted table DECOMPOSES its residue into the gaps between leaves refuses at CHECK(seconds >= 0.0). So the table would say so; nothing would pass quietly.

It is unreachable on the shipped path, and that is a property of the CALL SITE rather than of the function. Both WritePhaseLog calls in src/vllm/multimodal/ltx2_video.cpp run after generate_span.Close(); that span is the last live scope; and PhaseLog::Close stops and JOINS the sampler thread before it returns when nothing is left live. No thread can close a scope between those two lines on any path this project ships. A fresh review also failed to stage the inversion adversarially: 27,471 probes of a churn thread against a replica of the two statements produced zero inversions.

NOT FIXED IN FLOW: the real repair is to make the pair a single locked snapshot, and Elapsed() and Records() are separate public entry points on PhaseLog, so a combined one is a public API change. A public API change owes its own row, spec and red-first evidence rather than being smuggled into a repair commit for a different defect.

Owned by .agents/specs/ltx25-phase-instrument.md ## Owed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions