diff --git a/gems/aws-sdk-s3/CHANGELOG.md b/gems/aws-sdk-s3/CHANGELOG.md index 49c558bd128..89c093df61c 100644 --- a/gems/aws-sdk-s3/CHANGELOG.md +++ b/gems/aws-sdk-s3/CHANGELOG.md @@ -1,6 +1,8 @@ Unreleased Changes ------------------ +* Issue - Ensure the source file is closed on multipart `upload_file` part failure, preventing leaked file descriptors (#3408). + 1.228.0 (2026-07-16) ------------------ diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb index c65f4cd21ff..3f29be384c7 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb @@ -154,7 +154,6 @@ def upload_with_executor(pending, completed, options) Thread.current[:net_http_override_body_stream_chunk] = @http_chunk_size if @http_chunk_size update_progress(progress, p) resp = @client.upload_part(p) - p[:body].close completed_part = { etag: resp.etag, part_number: p[:part_number] } apply_part_checksum(resp, completed_part) completed.push(completed_part) @@ -162,6 +161,7 @@ def upload_with_executor(pending, completed, options) abort_upload = true errors << e ensure + p[:body].close Thread.current[:net_http_override_body_stream_chunk] = nil if @http_chunk_size completion_queue << :done end diff --git a/gems/aws-sdk-s3/spec/multipart_file_uploader_spec.rb b/gems/aws-sdk-s3/spec/multipart_file_uploader_spec.rb index 4901ff59361..23aea0c5800 100644 --- a/gems/aws-sdk-s3/spec/multipart_file_uploader_spec.rb +++ b/gems/aws-sdk-s3/spec/multipart_file_uploader_spec.rb @@ -125,6 +125,37 @@ module S3 expect { subject.upload(large_file, params) }.to raise_error(/multipart upload failed: part 3 failed/) end + it 'closes all file parts even when a part upload fails' do + # Fail the last part so the posting loop can't break early and skip + # un-posted parts. This keeps the assertion deterministic across MRI + # (GIL-serialized) and JRuby (truly parallel) threads. + file = Tempfile.new('six-meg-file').tap do |f| + 6.times { f.write(one_mb) } + f.rewind + end + file_parts = [] + allow(FilePart).to receive(:new).and_wrap_original do |original, *args| + original.call(*args).tap do |fp| + allow(fp).to receive(:close).and_call_original + file_parts << fp + end + end + + client.stub_responses( + :upload_part, + [ + { etag: 'etag-1' }, + RuntimeError.new('part 2 failed') + ] + ) + + expect { subject.upload(file, params) } + .to raise_error(/multipart upload failed: part 2 failed/) + + expect(file_parts.size).to eq(2) + file_parts.each { |fp| expect(fp).to have_received(:close) } + end + it 'reports when it is unable to abort a failed multipart upload', :jruby_flaky do client.stub_responses(:upload_part, RuntimeError.new('part failed')) client.stub_responses(:abort_multipart_upload, RuntimeError.new('network-error'))