schedule_store.acquire is the mutual-exclusion primitive that stops a scheduled
workflow from running twice at once. It is an unlocked read-modify-write, and two
scheduler ticks run concurrently in normal operation, so the lock it grants can
be granted to two ticks at the same time — each of which then runs the workflow.
The primitive's own comment states the guarantee: "A LIVE lock means another run
of THIS schedule is in progress. NEITHER 'skip' NOR 'queue' may run concurrently
— the lock exists precisely to prevent that." The implementation:
rec = load(ws, schedule_id) # jload
lock = rec.get("lock")
if lock and now < parse(lock["expires_at"]):
return False, rec, SKIPPED_OVERLAP # (A) check: a run is in progress
rec["lock"] = {run_id, acquired_at, expires_at}
return True, _save(ws, rec), reason # (B) set + jsave
Nothing serializes (A) and (B). Two threads that both load the record while
lock is None both pass the check and both write their own lock, and both return
ok=True.
Two tick threads exist in the same process. routes/schedules.run_due_now (the
tick that iterates due schedules and calls acquire on each) is driven from:
- the in-station clock thread — studio_server.py:7363 _scheduler_clock,
started at boot, calls run_due_now every SCHEDULER_TICK_SECONDS; and
- POST /api/schedules/tick — routes/schedules.py:493, called by the external
companion driver and the CLI on their own cadence.
run_due_now holds no outer lock (acquire IS the guard), so whenever the clock
tick and a driver tick overlap, both call acquire on the same schedule id and
both win. For a disposition: "auto" (live) schedule, both winners proceed to
_run_scheduled, which POSTs the workflow to the loopback /api/workflow/dag/run
with no shared run_id — two independent live runs of a single occurrence. A
scheduled payout, batch send, or refund fires twice.
There is a second, coarser symptom of the same unlocked design: schedule_store.
_save writes through a fixed "<id>.json.tmp" path and os.replace()s it into
place. Two concurrent saves race on that one temp path, so the losing tick's
os.replace raises FileNotFoundError (the temp file was already moved by the
other) — the tick thread dies with an unhandled error rather than double-running.
So concurrent ticks either double-execute (when the writes happen to interleave
cleanly) or crash the tick — never the clean single-run the lock promises.
Reproduction steps:
- Extract the station-v1.0.0 tarball to a clean directory; put workbench/ on
sys.path and import primitives.schedule_store.
- Create an auto-disposition schedule via schedule_store.create(...).
- SEQUENTIAL control: acquire twice in a row; confirm the second returns
ok=False, SKIPPED_OVERLAP.
- Reset lock to None, then call acquire from two threads. Wrap
schedule_store.load with a threading.Barrier(2) so both threads finish their
read before either save (the interleaving the two tick threads permit). To
observe both return values without the temp-path collision, serialize only
the _save call — this does not touch the check-then-set logic.
- Read the ok verdict of both threads.
Expected: exactly one tick acquires the lock for a given occurrence; the other
is refused with SKIPPED_OVERLAP, as in the sequential case.
Actual:
sequential -> [ok=True, ok=False (SKIPPED_OVERLAP)]
concurrent -> [ok=True, ok=True] (both hold the lock)
without the _save serialization, the two ticks instead crash on the shared
"<id>.json.tmp" path (FileNotFoundError in os.replace)
Root cause: an overlap-prevention lock is implemented as check-then-set across a
file read and a file write with no mutual exclusion, on a station that ticks the
scheduler from two threads (the in-station clock and the HTTP tick endpoint). The
lock's own read-modify-write is the critical section it fails to protect.
Suggested fix: guard the whole load-check-set-save in acquire under a process
lock (a module-level threading.Lock, keyed per schedule id if contention
matters), so the check and the set are atomic against a concurrent tick; and give
_save a unique temp filename (e.g. include os.getpid()+token) so a raced write
cannot delete another's temp file. run_due_now taking a single lock around each
per-schedule acquire+dispatch would also close it. An OS-level file lock
(fcntl.flock) on the schedule file is the cross-process-hardened version, since
the external driver runs in a separate process from the in-station clock.