ENG-9398: fix upload handler race condition#6368
Conversation
Greptile SummaryThis PR fixes a race condition in Confidence Score: 5/5Safe to merge — the fix is correct and well-tested; the only remaining finding is a minor asyncio warning in error paths. All remaining findings are P2 (style/cosmetic). The core race-condition fix is sound: the sentinel-in-queue pattern is a well-established approach, and the six new unit tests plus the regression test give strong confidence in edge-case coverage. The uv.lock additions are unrelated housekeeping. No files require special attention; the minor unhandled-exception warning in _stream_queue_until_done is cosmetic. Important Files Changed
Sequence DiagramsequenceDiagram
participant C as Caller
participant EP as enqueue_stream_delta
participant H as Handler Task (task_future)
participant Q as deltas Queue
participant S as _stream_queue_until_done
C->>EP: enqueue_stream_delta(token, event)
EP->>H: enqueue(event) → task_future
EP->>S: _stream_queue_until_done(queue=deltas, done_when=task_future.wait_all())
S->>S: create_task(done_when) → watcher
S->>S: watcher.add_done_callback(→ queue.put_nowait(sentinel))
H->>Q: emit_delta({i: 0}) → put(delta0)
S->>Q: await queue.get() → delta0
S-->>EP: yield delta0
EP-->>C: yield delta0
H->>Q: emit_delta({i: 1}) → put(delta1)
H->>H: complete (task_future done)
H-->>S: done_callback fires → queue.put_nowait(sentinel)
S->>Q: await queue.get() → delta1
S-->>EP: yield delta1
EP-->>C: yield delta1
S->>Q: await queue.get() → sentinel
S->>S: return (end of stream)
EP->>H: await task_future.wait_all() (re-raise any exception)
EP-->>C: stream complete
Reviews (2): Last reviewed commit: "make helper and write tests" | Re-trigger Greptile |
|
@greptile |
No description provided.