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.
What's wrong
JobService._move(src/visionset/kernel/services/job_service.py:415, used bystart,completeandstart_if_pending) reads the job once inside itsunit_of_work(), decides legality from that one read, and writes a blinduow.annotation_jobs.update(job.model_copy(...))at the end of the sametransaction.
unit_of_work()does not take a lock at open — pysqlite defersBEGINto the first write statement — so the read this method starts fromis 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.updateis a plainsession.merge(), no compare-and-swap).Repro
Hold a
start/start_if_pending/completecall open right after its read(any override of
require_open_batchorrequire_jobthat blocks therereproduces this deterministically). While it's held, on a second connection
over the same workspace:
startthe job,markevery assetskipped, thencompleteit — 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-
completedjob back toin_progress. The per-assetprogressmap is unaffected (children arewritten separately and are not part of the blind merge), but the job's own
statefield goes backwards.Why this matters
docs/content/jobs.mdandrequire_open_job(
src/visionset/kernel/services/job_service.py, theJobFinishedguard) bothsay a completed job does not re-open. This lets it happen anyway, silently —
no exception, no refusal, just a
completedjob that readsin_progressagain afterward.
Scope
Found while adding
JobService.start_if_pendingfor #884's autostart-racefinding;
start_if_pendingdoes not introduce this — the same repro againstplain
JobService.startreproduces it identically, so this is pre-existing in_moveand affects every caller (start,complete,start_if_pending).Direction
_move's write needs to check it's still updating the row it read — the samecompare-and-swap
set_asset_progressalready uses for per-asset progress(
uow.set_asset_progress(..., expected=current, ...)), applied to the job'sown
statecolumn instead of a blindannotation_jobs.update.