fix(client): non-blocking fingerprint + immediate stop_event cancel (async)#2
Merged
Merged
Conversation
…async) Address gemini-code-assist review on #1: - Run the SHA-256 fingerprint via asyncio.to_thread so a large file's hashing no longer blocks the event loop (only computed when store_url is set). - Check stop_event at the top of AsyncUploader.upload_chunk and in the upload loop, so cancellation works even with retries disabled (max_retries=0) or while chunks keep succeeding — previously it was only honored during the retry backoff wait. Client-side only; no wire/header/offset behavior changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rint Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The async dispatch held a threading.Lock across `await cleanup_expired_uploads_async()`. With `cleanup_interval <= 0` the double-checked guard always passes, so a second concurrent request would call `acquire()` and block the event loop thread — a hard deadlock, since the awaiting coroutine can never resume to release it. Replace the lock with a non-blocking `_cleanup_running` flag on the async path only; the sync path keeps the lock (no await, lock is correct there). Single event loop => a plain flag is enough to prevent overlapping runs. Regression test runs the loop in a worker thread and joins with a timeout, since a frozen loop can't honor an in-loop asyncio.wait_for. Addresses gemini-code-assist critical comment on #1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1, addressing the actionable gemini-code-assist comments.
Changes
AsyncTusClientnow computes the SHA-256 fingerprint viaasyncio.to_thread, so hashing a large file no longer blocks the loop (only runs whenstore_urlis set).stop_eventcancellation —AsyncUploadernow checksstop_eventat the top ofupload_chunkand inside theuploadloop. Previously it was only honored during the retry backoff wait, so withmax_retries=0(default) or while chunks kept succeeding, cancellation was ignored.test_async_uploader_stop_event_cancels_without_retries.Not addressed (intentional)
_last_cleanupis updated before theawait, so the double-checked guard makes concurrent requests skip the lock entirely in a single event loop. No change.parallel.py); a streamingFileSliceis a larger change for both paths, out of scope here.TUS compliance
Client-side only. No wire bytes, headers, status codes, or
Upload-Offsetsemantics change. Astop_eventcancellation raises before sending a PATCH, leaving the partial upload resumable per the protocol.Test result
pytest— 495 passed, 4 skippedruff check/ruff format --check/ty checkclean🤖 Generated with Claude Code