Skip to content

Fix rare race condition in multipart upload with low maxInFlightParts#7172

Merged
alextwoods merged 3 commits into
masterfrom
alexwoo/fix-mpu-stale-inflight-race
Jul 22, 2026
Merged

Fix rare race condition in multipart upload with low maxInFlightParts#7172
alextwoods merged 3 commits into
masterfrom
alexwoo/fix-mpu-stale-inflight-race

Conversation

@alextwoods

Copy link
Copy Markdown
Contributor

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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have read the CONTRIBUTING document
  • Local run of mvn install succeeds
  • My code follows the code style of this project
  • My change requires a change to the Javadoc documentation
  • I have updated the Javadoc documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed
  • I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.

License

  • I confirm that this pull request can be released under the Apache 2 license

…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.
@alextwoods
alextwoods requested a review from a team as a code owner July 21, 2026 17:12
@alextwoods
alextwoods requested a review from S-Saranya1 July 21, 2026 19:09
@alextwoods
alextwoods added this pull request to the merge queue Jul 22, 2026
Merged via the queue into master with commit f09a14c Jul 22, 2026
12 of 13 checks passed
@github-actions

Copy link
Copy Markdown

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.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jul 22, 2026
@alextwoods
alextwoods deleted the alexwoo/fix-mpu-stale-inflight-race branch July 22, 2026 17:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants