From f6e23d5537c7430f128a28f129f85597a00cc98a Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Tue, 21 Jul 2026 17:43:03 -0700 Subject: [PATCH 1/2] Ensure source file is closed on multipart upload_file failure --- gems/aws-sdk-s3/CHANGELOG.md | 2 ++ .../lib/aws-sdk-s3/multipart_file_uploader.rb | 2 +- .../spec/multipart_file_uploader_spec.rb | 26 +++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) 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..e22648b4dc2 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,32 @@ 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 + 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'), + { etag: 'etag-3' }, + { etag: 'etag-4' } + ] + ) + + expect { subject.upload(large_file, params) } + .to raise_error(/multipart upload failed: part 2 failed/) + + expect(file_parts).not_to be_empty + 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')) From 446c9914c4e5e09c61b8bd661cb46147615d7541 Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Wed, 22 Jul 2026 09:00:27 -0700 Subject: [PATCH 2/2] Make MultipartFileUploader close-on-failure spec deterministic --- .../spec/multipart_file_uploader_spec.rb | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) 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 e22648b4dc2..23aea0c5800 100644 --- a/gems/aws-sdk-s3/spec/multipart_file_uploader_spec.rb +++ b/gems/aws-sdk-s3/spec/multipart_file_uploader_spec.rb @@ -126,6 +126,13 @@ module S3 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| @@ -138,16 +145,14 @@ module S3 :upload_part, [ { etag: 'etag-1' }, - RuntimeError.new('part 2 failed'), - { etag: 'etag-3' }, - { etag: 'etag-4' } + RuntimeError.new('part 2 failed') ] ) - expect { subject.upload(large_file, params) } + expect { subject.upload(file, params) } .to raise_error(/multipart upload failed: part 2 failed/) - expect(file_parts).not_to be_empty + expect(file_parts.size).to eq(2) file_parts.each { |fp| expect(fp).to have_received(:close) } end