Fix rare race condition in multipart upload with low maxInFlightParts#7172
Merged
Conversation
…htParts The multipart upload subscribers decided whether to initiate CompleteMultipartUpload using a stale snapshot of the in-flight part counter taken by decrementAndGet() in the upload completion callback. Between that snapshot and the completion check, subscription.request(1) can synchronously deliver the final AsyncRequestBody (starting a new upload) followed by onComplete() (setting isDone), because SimplePublisher delivers queued signals on the requesting thread. The callback then saw isDone == true with its stale count of 0 and initiated CompleteMultipartUpload while the final part was still in flight. For unknown content length this failed uploads with 'The number of UploadParts requests is not equal to the expected number of parts. Expected: N, Actual: N-1'. For known content length the part-count validation passed (partNumber was already incremented by the final onNext) and CompleteMultipartUpload was sent with a null part entry. The window only opens when delivery of the final body is gated on completion callbacks' request(1) calls, which is why it was observed with maxInFlightParts=2 but not with the default of 50, and only intermittently under real network timing. Fix: re-read asyncRequestBodyInFlight inside the completion check instead of trusting the caller's snapshot. Once the volatile isDone is observed true, all onNext increments are visible, so a fresh read of 0 guarantees every started upload has finished. Both regression tests script the exact delivery order with a SimplePublisher-faithful Subscription and fail without the fix.
S-Saranya1
approved these changes
Jul 21, 2026
zoewangg
approved these changes
Jul 21, 2026
|
This pull request has been closed and the conversation has been locked. Comments on closed PRs are hard for our team to see. If you need more assistance, please open a new issue that references this one. |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fix rare race condition in multipart upload with low maxInFlightParts
Motivation and Context
With maxInFlightParts=2 and the unknown-content-length upload path, multipart uploads can very rarely fail with
SdkClientException: The number of UploadParts requests is not equal to the expected number of parts. Expected: N, Actual: N-1.The upload completion callback in both MPU subscribers takes a snapshot via asyncRequestBodyInFlight.decrementAndGet(), then calls subscription.request(1), then decides completion using the stale snapshot. SimplePublisher delivers queued signals synchronously on the requesting thread, so that request(1) can deliver the final part body (starting a new upload) followed by onComplete() (setting isDone) inside the window. The callback then sees isDone == true with its stale count of 0 and initiates CompleteMultipartUpload while the final part is still in flight. The known-content-length subscriber has the same window but passes its part-count validation, sending CompleteMultipartUpload with a null part entry.
Modifications
re-read InFlight where it is used instead of trusting the caller's snapshot. Since isDone is volatile and written after all serialized onNext increments, observing it true guarantees the increments are visible, so a fresh read of 0 means every started upload has finished.
Testing
Added new regression tests
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License