summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKamil Trzciński <ayufan@ayufan.eu>2018-11-23 16:46:33 +0100
committerKamil Trzciński <ayufan@ayufan.eu>2018-11-26 13:15:46 +0100
commitc150772edb65f727c9be0a82ec71c4b084c9d949 (patch)
treeabef5c3b6eedd400075dd322b096638041328e1f
parent2e3dab38295b7c36ab100f20c654fdfaf9b00885 (diff)
downloadgitlab-ce-fix-deadlock-chunked-io.tar.gz
Fix deadlock on ChunkedIOfix-deadlock-chunked-io
-rw-r--r--changelogs/unreleased/fix-deadlock-chunked-io.yml5
-rw-r--r--lib/gitlab/ci/trace/chunked_io.rb29
-rw-r--r--lib/gitlab/http_io.rb8
-rw-r--r--spec/lib/gitlab/ci/trace/chunked_io_spec.rb63
4 files changed, 88 insertions, 17 deletions
diff --git a/changelogs/unreleased/fix-deadlock-chunked-io.yml b/changelogs/unreleased/fix-deadlock-chunked-io.yml
new file mode 100644
index 00000000000..def7a59e86e
--- /dev/null
+++ b/changelogs/unreleased/fix-deadlock-chunked-io.yml
@@ -0,0 +1,5 @@
+---
+title: Fix deadlock on ChunkedIO
+merge_request:
+author:
+type: fixed
diff --git a/lib/gitlab/ci/trace/chunked_io.rb b/lib/gitlab/ci/trace/chunked_io.rb
index e9b3199d56e..8c6fd56493f 100644
--- a/lib/gitlab/ci/trace/chunked_io.rb
+++ b/lib/gitlab/ci/trace/chunked_io.rb
@@ -13,7 +13,7 @@ module Gitlab
attr_reader :build
attr_reader :tell, :size
- attr_reader :chunk, :chunk_range
+ attr_reader :chunk_data, :chunk_range
alias_method :pos, :tell
@@ -75,14 +75,14 @@ module Gitlab
until length <= 0 || eof?
data = chunk_slice_from_offset
- break if data.empty?
+ raise FailedToGetChunkError if data.empty?
chunk_bytes = [CHUNK_SIZE - chunk_offset, length].min
- chunk_data = data.byteslice(0, chunk_bytes)
+ chunk_data_slice = data.byteslice(0, chunk_bytes)
- out << chunk_data
- @tell += chunk_data.bytesize
- length -= chunk_data.bytesize
+ out << chunk_data_slice
+ @tell += chunk_data_slice.bytesize
+ length -= chunk_data_slice.bytesize
end
out = out.join
@@ -100,11 +100,14 @@ module Gitlab
until eof?
data = chunk_slice_from_offset
+ raise FailedToGetChunkError if data.empty?
+
new_line = data.index("\n")
if !new_line.nil?
- out << data[0..new_line]
- @tell += new_line + 1
+ raw_data = data[0..new_line]
+ out << raw_data
+ @tell += raw_data.bytesize
break
else
out << data
@@ -121,13 +124,13 @@ module Gitlab
while tell < start_pos + data.bytesize
# get slice from current offset till the end where it falls into chunk
chunk_bytes = CHUNK_SIZE - chunk_offset
- chunk_data = data.byteslice(tell - start_pos, chunk_bytes)
+ data_slice = data.byteslice(tell - start_pos, chunk_bytes)
# append data to chunk, overwriting from that point
- ensure_chunk.append(chunk_data, chunk_offset)
+ ensure_chunk.append(data_slice, chunk_offset)
# move offsets within buffer
- @tell += chunk_data.bytesize
+ @tell += data_slice.bytesize
@size = [size, tell].max
end
@@ -183,12 +186,12 @@ module Gitlab
current_chunk.tap do |chunk|
raise FailedToGetChunkError unless chunk
- @chunk = chunk.data
+ @chunk_data = chunk.data
@chunk_range = chunk.range
end
end
- @chunk[chunk_offset..CHUNK_SIZE]
+ @chunk_data.byteslice(chunk_offset, CHUNK_SIZE)
end
def chunk_offset
diff --git a/lib/gitlab/http_io.rb b/lib/gitlab/http_io.rb
index e768b8adb12..6a9fb85b054 100644
--- a/lib/gitlab/http_io.rb
+++ b/lib/gitlab/http_io.rb
@@ -85,11 +85,11 @@ module Gitlab
break if data.empty?
chunk_bytes = [BUFFER_SIZE - chunk_offset, length].min
- chunk_data = data.byteslice(0, chunk_bytes)
+ data_slice = data.byteslice(0, chunk_bytes)
- out << chunk_data
- @tell += chunk_data.bytesize
- length -= chunk_data.bytesize
+ out << data_slice
+ @tell += data_slice.bytesize
+ length -= data_slice.bytesize
end
out = out.join
diff --git a/spec/lib/gitlab/ci/trace/chunked_io_spec.rb b/spec/lib/gitlab/ci/trace/chunked_io_spec.rb
index 6259b952add..546a9e7d0cc 100644
--- a/spec/lib/gitlab/ci/trace/chunked_io_spec.rb
+++ b/spec/lib/gitlab/ci/trace/chunked_io_spec.rb
@@ -116,6 +116,19 @@ describe Gitlab::Ci::Trace::ChunkedIO, :clean_gitlab_redis_cache do
chunked_io.each_line { |line| }
end
end
+
+ context 'when buffer consist of many empty lines' do
+ let(:sample_trace_raw) { Array.new(10, " ").join("\n") }
+
+ before do
+ build.trace.set(sample_trace_raw)
+ end
+
+ it 'yields lines' do
+ expect { |b| chunked_io.each_line(&b) }
+ .to yield_successive_args(*string_io.each_line.to_a)
+ end
+ end
end
context "#read" do
@@ -143,6 +156,22 @@ describe Gitlab::Ci::Trace::ChunkedIO, :clean_gitlab_redis_cache do
end
end
+ context 'when chunk is missing data' do
+ let(:length) { nil }
+
+ before do
+ stub_buffer_size(1024)
+ build.trace.set(sample_trace_raw)
+
+ # make second chunk to not have data
+ build.trace_chunks.second.append('', 0)
+ end
+
+ it 'raises an error' do
+ expect { subject }.to raise_error described_class::FailedToGetChunkError
+ end
+ end
+
context 'when read only first 100 bytes' do
let(:length) { 100 }
@@ -266,6 +295,40 @@ describe Gitlab::Ci::Trace::ChunkedIO, :clean_gitlab_redis_cache do
expect(chunked_io.readline).to eq(string_io.readline)
end
end
+
+ context 'when chunk is missing data' do
+ let(:length) { nil }
+
+ before do
+ build.trace.set(sample_trace_raw)
+
+ # make first chunk to have invalid data
+ build.trace_chunks.first.append('data', 0)
+ end
+
+ it 'raises an error' do
+ expect { subject }.to raise_error described_class::FailedToGetChunkError
+ end
+ end
+
+ context 'when utf-8 is being used' do
+ let(:sample_trace_raw) { sample_trace_raw_utf8.force_encoding(Encoding::BINARY) }
+ let(:sample_trace_raw_utf8) { "😺\n😺\n😺\n😺" }
+
+ before do
+ stub_buffer_size(3) # the utf-8 character has 4 bytes
+
+ build.trace.set(sample_trace_raw_utf8)
+ end
+
+ it 'has known length' do
+ expect(sample_trace_raw_utf8.bytesize).to eq(4 * 4 + 3 * 1)
+ expect(sample_trace_raw.bytesize).to eq(4 * 4 + 3 * 1)
+ expect(chunked_io.size).to eq(4 * 4 + 3 * 1)
+ end
+
+ it_behaves_like 'all line matching'
+ end
end
context "#write" do