Skip to content

A completed job can be silently reopened by a same-transaction stale read #885

Description

@JArmandoAnaya

What's wrong

JobService._move (src/visionset/kernel/services/job_service.py:415, used by
start, complete and start_if_pending) reads the job once inside its
unit_of_work(), decides legality from that one read, and writes a blind
uow.annotation_jobs.update(job.model_copy(...)) at the end of the same
transaction. unit_of_work() does not take a lock at open — pysqlite defers
BEGIN to the first write statement — so the read this method starts from
is not a snapshot: it can go stale while the transaction is still open, and
the final write does not check that the row is still what was read
(SqlRepository.update is a plain session.merge(), no compare-and-swap).

Repro

Hold a start/start_if_pending/complete call open right after its read
(any override of require_open_batch or require_job that blocks there
reproduces this deterministically). While it's held, on a second connection
over the same workspace: start the job, mark every asset skipped, then
complete it — this fully completes the job. Release the held call.

Observed: the held call's write still lands, using the state it read before
any of that happened, and moves the now-completed job back to
in_progress. The per-asset progress map is unaffected (children are
written separately and are not part of the blind merge), but the job's own
state field goes backwards.

Why this matters

docs/content/jobs.md and require_open_job
(src/visionset/kernel/services/job_service.py, the JobFinished guard) both
say a completed job does not re-open. This lets it happen anyway, silently —
no exception, no refusal, just a completed job that reads in_progress
again afterward.

Scope

Found while adding JobService.start_if_pending for #884's autostart-race
finding; start_if_pending does not introduce this — the same repro against
plain JobService.start reproduces it identically, so this is pre-existing in
_move and affects every caller (start, complete, start_if_pending).

Direction

_move's write needs to check it's still updating the row it read — the same
compare-and-swap set_asset_progress already uses for per-asset progress
(uow.set_asset_progress(..., expected=current, ...)), applied to the job's
own state column instead of a blind annotation_jobs.update.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    backendvisionset Python distribution — kernel, server, cli, mcp, formatskernelvisionset.kernel — domain, ports, adapters, services

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions