fix(queue): use a thread lock for JobQueue across event loops

[SebTardif]

What Problem This Solves

The Hugging Face Space UI submits jobs with asyncio.run(queue.submit()) on a brand-new event loop. The background EvalWorker already owns JobQueue._lock on a different loop. That lock was an asyncio.Lock, which is bound to the loop that first used it.

When a visitor clicks Submit while the worker is claiming a job or writing queue state, the public submit path can hang until the Space is restarted, or it can raise that the lock belongs to another loop. The form looks stuck even though the request already reached submit_model.

This change switches JobQueue to a threading.Lock so Gradio's per-click asyncio.run() and the worker loop can share the same queue. The lock now covers only in-memory mutation and the local jobs.json write. Hub uploads run after release, so a slow Hugging Face dataset push cannot pin the Space submit button.

Evidence

Public call chain: Gradio submit_model / submit_all_presets in app.py call asyncio.run(queue.submit()). The worker thread calls claim_pending and update_progress on its own loop. Both paths used the same asyncio.Lock.

Before (same two-asyncio.run() shape as Space submit vs worker):

$ python /tmp/shellbench-F003-before.py
lock_type=asyncio.locks.Lock
submit_alive=True
submit_elapsed_ms=2059.4
submit_got=None
FAIL: second asyncio.run could not take the loop-bound lock

After (patched JobQueue, same asyncio.run(queue.submit()) as app.py):

$ python /tmp/shellbench-F003-proof.py
queue_dir=/var/folders/7_/3g02szlx2h3_4pdqp0knlw740000gn/T/clawbench-f003-p465nicg
lock_type=_thread.lock
submit_job_id=8ab02a48
submit_status=pending
submit_elapsed_ms=56.1
jobs_file_exists=True
OK: second-loop asyncio.run(submit) returned after worker released thread lock
overlap_job_id=eca5853f
overlap_model=huggingface/zai-org/GLM-5
overlap_elapsed_ms=0.5
OK: asyncio.run(submit) returned while a prior hub sync was still in flight

The lock has been an asyncio.Lock since the initial queue in 1df8c43 (2026-04-07). No open pull request already changes this file for the same hang.

Real behavior proof

• Behavior or issue addressed: Space Submit can hang or raise when asyncio.run(queue.submit()) runs on a new loop while the eval worker holds JobQueue._lock on another loop.

• Real environment tested: macOS Darwin 25.6.0 arm64, Python 3.14.7, checkout /tmp/oc-pr-shellbench-F003 at branch fix/f003-jobqueue-thread-lock, no HF_TOKEN.

• Exact steps or command run after this patch:

  python /tmp/shellbench-F003-before.py
  python /tmp/shellbench-F003-proof.py

• Evidence after fix: terminal output from the patched tree:

  lock_type=_thread.lock
  submit_job_id=8ab02a48
  submit_status=pending
  submit_elapsed_ms=56.1
  jobs_file_exists=True
  OK: second-loop asyncio.run(submit) returned after worker released thread lock
  overlap_job_id=eca5853f
  overlap_elapsed_ms=0.5
  OK: asyncio.run(submit) returned while a prior hub sync was still in flight

• Observed result after fix: JobQueue._lock is _thread.lock. A second-loop asyncio.run(submit) returned a pending job in 56.1ms after the worker released the lock. A later asyncio.run(submit) returned in 0.5ms while a previous hub sync was still blocked, so the lock is not held across uploads.

• What was not tested: A live Hugging Face Space with a real dataset upload, Gradio click-through, or an in-progress native eval on the public Space.

Changes

• Replace asyncio.Lock with threading.Lock on JobQueue.
• Persist local queue state under the lock, then release before _sync_to_hub().
• Cover the Space two-loop submit path and the in-flight hub-upload overlap.

python -m ruff check clawbench/queue.py tests/test_queue.py passed on the changed files.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: blocked before merge. Reviewed September 3, 2026, 5:51 AM ET / 09:51 UTC.

ClawSweeper review

What this changes

The PR replaces JobQueue’s event-loop-bound lock with a thread lock, releases it before Hugging Face dataset synchronization, and adds cross-event-loop queue tests.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

⛔ Blocked before merge - 5 items remain

The cross-event-loop lock fix is useful and its supplied terminal trace demonstrates the intended path, but this unchanged head still allows concurrent remote queue uploads to complete out of order and restore stale job state. The prior P1 finding therefore remains a merge blocker.

Priority: P1
Reviewed head: 905dd76213e8dd3b8c2a5f6dccd663b58d62e459

Review scores

Measure	Result	What it means
Overall readiness	🧂 unranked krab (1/6)	The main liveness fix is demonstrated, but a P1 persistence-ordering regression prevents merge readiness.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The PR body supplies an after-fix macOS terminal trace for the changed JobQueue path: a second asyncio.run(queue.submit()) returns after another loop releases the new thread lock, and a later submission returns while an earlier synchronization is blocked. This is real behavior proof for the cross-loop liveness claim, although it does not cover the separate remote publication-ordering defect.
Patch quality	🧂 unranked krab (1/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The PR body supplies an after-fix macOS terminal trace for the changed JobQueue path: a second asyncio.run(queue.submit()) returns after another loop releases the new thread lock, and a later submission returns while an earlier synchronization is blocked. This is real behavior proof for the cross-loop liveness claim, although it does not cover the separate remote publication-ordering defect.
Evidence reviewed	5 items	Introduced concurrent publication: This PR moves queue publication outside the new thread lock, allowing another queue transition to mutate and save a newer jobs.json while an earlier publication remains in flight. Remote state is a single overwrite target: Every synchronization submits the same local jobs.json to queue/jobs.json, and startup overlays remote rows onto locally loaded jobs, so an older completion can restore stale status or progress for an existing job. Prior finding remains at the exact head: The previous completed review identified un-serialized Hugging Face snapshot uploads. The checked-out head is the same reviewed SHA and has no later diff in either changed file, so that blocker has not been addressed.
Findings	1 actionable finding	[P1] Serialize Hugging Face queue snapshot uploads
Security	None	None.

How this fits together

JobQueue accepts submissions from the Gradio Space and claims/progress updates from the background evaluation worker. It saves queue state locally and publishes a shared snapshot to a Hugging Face dataset so job state can be restored after restart.

flowchart LR
UI[Space submission] --> Queue[Job queue]
Worker[Evaluation worker] --> Queue
Queue --> Save[Locked local snapshot]
Save --> Upload[Dataset snapshot upload]
Upload --> Dataset[Hugging Face dataset]
Dataset --> Recovery[Restart recovery]

Loading

Before merge

• Serialize Hugging Face queue snapshot uploads (P1) - Releasing the queue lock before this call allows two transitions to upload the same queue/jobs.json concurrently. An older snapshot can complete last and overwrite newer status or progress, which _load_hub() then applies on restart. The overlap test proves liveness only; serialize or version publication and assert delayed older completion cannot win.
• Resolve merge risk (P1) - Concurrent uploads of the same remote queue snapshot can finish out of order, allowing an older status or progress snapshot to overwrite a newer one and be re-applied during restart recovery.
• Complete next step (P2) - Serialize or revision-gate Hugging Face queue snapshot publication, then add a delayed-completion test proving restart recovery keeps the newest queue state.
• Improve patch quality - Serialize or revision-gate queue dataset publication so stale snapshots cannot complete last.
• Improve patch quality - Add a delayed-completion regression test proving restart recovery retains the newest queue transition.

Findings

• [P1] Serialize Hugging Face queue snapshot uploads — clawbench/queue.py:237

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and regression-test delta	production +8 net, tests +117	The compact production change unlocks concurrent remote writes, while the added tests cover liveness but not publication ordering.

Merge-risk options

Maintainer options:

1. Serialize ordered snapshot publication (recommended)
   Keep queue mutation responsive, but serialize or revision-gate Hugging Face publication and add a delayed-completion test proving restart recovery retains the newest transition.
2. Accept stale recovery risk
   Merge the cross-loop responsiveness fix while accepting that a delayed older upload can overwrite the remote restart snapshot.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Serialize or revision-gate Hugging Face queue snapshot publication without holding the mutation lock across I/O, and add a delayed-completion regression test for newest-state restart recovery.

Technical review

Best possible solution:

Keep the fast cross-loop mutation path, but assign ordered snapshots under the queue lock and serialize or version publication so only the newest Hugging Face snapshot can become the restart source of truth; cover delayed old-upload completion.

Do we have a high-confidence way to reproduce the issue?

Yes. A deterministic delayed first dataset upload followed by a later queue transition can make the later upload complete first and then let the captured older snapshot overwrite queue/jobs.json; startup then applies that remote row after local load.

Is this the best way to solve the issue?

No. A thread lock resolves the cross-event-loop lock ownership problem, but releasing all uploads without ordered publication creates a stale persistent-state path; serialized or revision-gated snapshots are the narrower safe solution.

Full review comments:

• [P1] Serialize Hugging Face queue snapshot uploads — clawbench/queue.py:237
  Releasing the queue lock before this call allows two transitions to upload the same queue/jobs.json concurrently. An older snapshot can complete last and overwrite newer status or progress, which _load_hub() then applies on restart. The overlap test proves liveness only; serialize or version publication and assert delayed older completion cannot win.
  Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against c1a79f731541.

Labels

Label justifications:

• P1: An ordinary Space submission or worker transition can cause persisted job status or progress to regress after restart.
• merge-risk: 🚨 session-state: The introduced concurrent synchronization path can make the remote restart snapshot stale relative to a later queue transition.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦞 diamond lobster and patch quality is 🧂 unranked krab.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body supplies an after-fix macOS terminal trace for the changed JobQueue path: a second asyncio.run(queue.submit()) returns after another loop releases the new thread lock, and a later submission returns while an earlier synchronization is blocked. This is real behavior proof for the cross-loop liveness claim, although it does not cover the separate remote publication-ordering defect.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies an after-fix macOS terminal trace for the changed JobQueue path: a second asyncio.run(queue.submit()) returns after another loop releases the new thread lock, and a later submission returns while an earlier synchronization is blocked. This is real behavior proof for the cross-loop liveness claim, although it does not cover the separate remote publication-ordering defect.

Evidence

Acceptance criteria:

• [P1] python -m pytest tests/test_queue.py.
• [P1] python -m ruff check clawbench/queue.py tests/test_queue.py.

What I checked:

• Introduced concurrent publication: This PR moves queue publication outside the new thread lock, allowing another queue transition to mutate and save a newer jobs.json while an earlier publication remains in flight. (clawbench/queue.py:237, 905dd76213e8)
• Remote state is a single overwrite target: Every synchronization submits the same local jobs.json to queue/jobs.json, and startup overlays remote rows onto locally loaded jobs, so an older completion can restore stale status or progress for an existing job. (clawbench/queue.py:149, 905dd76213e8)
• Prior finding remains at the exact head: The previous completed review identified un-serialized Hugging Face snapshot uploads. The checked-out head is the same reviewed SHA and has no later diff in either changed file, so that blocker has not been addressed. (clawbench/queue.py:237, 905dd76213e8)
• Current main still has the original cross-loop limitation: Current main retains asyncio.Lock in JobQueue, while the Space handlers invoke queue.submit through asyncio.run; this PR’s central lock fix is not already implemented on main. (clawbench/queue.py:106, c1a79f731541)
• Queue feature history: Feature history identifies the initial ClawBench queue commit as the earliest available queue implementation provenance; later queue hardening is shared across several contributors. (clawbench/queue.py:104, 1df8c430f3da)

Likely related people:

• scoootscooob: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (5 earlier review cycles)

• reviewed 2026-08-29T18:23:08.304Z sha 905dd76 :: needs changes before merge. :: [P1] Serialize dataset uploads before allowing another snapshot
• reviewed 2026-09-01T05:57:21.814Z sha 905dd76 :: needs changes before merge. :: [P1] Serialize dataset uploads before allowing another snapshot
• reviewed 2026-09-01T10:06:53.652Z sha 905dd76 :: needs changes before merge. :: [P1] Serialize Hugging Face snapshot uploads
• reviewed 2026-09-02T14:09:36.332Z sha 905dd76 :: needs changes before merge. :: [P1] Serialize Hugging Face snapshot uploads
• reviewed 2026-09-02T23:00:35.973Z sha 905dd76 :: blocked before merge. :: [P1] Serialize Hugging Face queue snapshot uploads