Skip to content

Commit 6b63cce

Browse files
phjordansissel
authored andcommitted
Fix a synchronization issue when in the `#teardown
Multiple threads were accessing the tempfile when we were trying to shutdown the pipeline. This issues was the cause of a non deterministic test for testing the file rotation. Fixes #23 Fixes #27
1 parent 76273ef commit 6b63cce

File tree

3 files changed

+13
-18
lines changed

3 files changed

+13
-18
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
# 1.0.1
22
- Fix a synchronization issue when doing file rotation and checking the size of the current file
3+
- Fix an issue with synchronization when shutting down the plugin and closing the current temp file

lib/logstash/outputs/s3.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,9 @@ def teardown
313313
shutdown_upload_workers
314314
@periodic_rotation_thread.stop! if @periodic_rotation_thread
315315

316-
@tempfile.close
316+
@file_rotation_lock.synchronize do
317+
@tempfile.close unless @tempfile.nil? && @tempfile.closed?
318+
end
317319
finished
318320
end
319321

spec/outputs/s3_spec.rb

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
# We stub all the calls from S3, for more information see:
1313
# http://ruby.awsblog.com/post/Tx2SU6TYJWQQLC3/Stubbing-AWS-Responses
1414
AWS.stub!
15-
1615
Thread.abort_on_exception = true
1716
end
1817

@@ -186,7 +185,6 @@
186185
end
187186

188187
context "having periodic rotations" do
189-
190188
let(:s3) { LogStash::Outputs::S3.new(minimal_settings.merge({ "size_file" => 1024, "time_file" => 6e-10 })) }
191189
let(:tmp) { Tempfile.new('s3_rotation_temp_file') }
192190

@@ -196,52 +194,46 @@
196194
end
197195

198196
after(:each) do
199-
tmp.close
197+
s3.teardown
198+
tmp.close
200199
tmp.unlink
201200
end
202201

203202
it "raises no error when periodic rotation happen" do
204203
1000.times do
205-
expect{ s3.rotate_events_log?}.not_to raise_error
204+
expect { s3.rotate_events_log? }.not_to raise_error
206205
end
207206
end
208-
209207
end
210208
end
211209

212210
describe "#move_file_to_bucket" do
213-
let!(:s3) { LogStash::Outputs::S3.new(minimal_settings) }
214-
215-
before do
216-
# Assume the AWS test credentials pass.
217-
allow(s3).to receive(:test_s3_write)
218-
s3.register
219-
end
211+
subject { LogStash::Outputs::S3.new(minimal_settings) }
220212

221213
it "should always delete the source file" do
222214
tmp = Stud::Temporary.file
223215

224216
allow(File).to receive(:zero?).and_return(true)
225217
expect(File).to receive(:delete).with(tmp)
226218

227-
s3.move_file_to_bucket(tmp)
219+
subject.move_file_to_bucket(tmp)
228220
end
229221

230222
it 'should not upload the file if the size of the file is zero' do
231223
temp_file = Stud::Temporary.file
232224
allow(temp_file).to receive(:zero?).and_return(true)
233225

234-
expect(s3).not_to receive(:write_on_bucket)
235-
s3.move_file_to_bucket(temp_file)
226+
expect(subject).not_to receive(:write_on_bucket)
227+
subject.move_file_to_bucket(temp_file)
236228
end
237229

238230
it "should upload the file if the size > 0" do
239231
tmp = Stud::Temporary.file
240232

241233
allow(File).to receive(:zero?).and_return(false)
242-
expect(s3).to receive(:write_on_bucket)
234+
expect(subject).to receive(:write_on_bucket)
243235

244-
s3.move_file_to_bucket(tmp)
236+
subject.move_file_to_bucket(tmp)
245237
end
246238
end
247239

0 commit comments

Comments
 (0)