Commit c150772e authored by Kamil Trzciński's avatar Kamil Trzciński

Fix deadlock on ChunkedIO

parent 2e3dab38
---
title: Fix deadlock on ChunkedIO
merge_request:
author:
type: fixed
...@@ -13,7 +13,7 @@ module Gitlab ...@@ -13,7 +13,7 @@ module Gitlab
attr_reader :build attr_reader :build
attr_reader :tell, :size attr_reader :tell, :size
attr_reader :chunk, :chunk_range attr_reader :chunk_data, :chunk_range
alias_method :pos, :tell alias_method :pos, :tell
...@@ -75,14 +75,14 @@ module Gitlab ...@@ -75,14 +75,14 @@ module Gitlab
until length <= 0 || eof? until length <= 0 || eof?
data = chunk_slice_from_offset data = chunk_slice_from_offset
break if data.empty? raise FailedToGetChunkError if data.empty?
chunk_bytes = [CHUNK_SIZE - chunk_offset, length].min 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 out << chunk_data_slice
@tell += chunk_data.bytesize @tell += chunk_data_slice.bytesize
length -= chunk_data.bytesize length -= chunk_data_slice.bytesize
end end
out = out.join out = out.join
...@@ -100,11 +100,14 @@ module Gitlab ...@@ -100,11 +100,14 @@ module Gitlab
until eof? until eof?
data = chunk_slice_from_offset data = chunk_slice_from_offset
raise FailedToGetChunkError if data.empty?
new_line = data.index("\n") new_line = data.index("\n")
if !new_line.nil? if !new_line.nil?
out << data[0..new_line] raw_data = data[0..new_line]
@tell += new_line + 1 out << raw_data
@tell += raw_data.bytesize
break break
else else
out << data out << data
...@@ -121,13 +124,13 @@ module Gitlab ...@@ -121,13 +124,13 @@ module Gitlab
while tell < start_pos + data.bytesize while tell < start_pos + data.bytesize
# get slice from current offset till the end where it falls into chunk # get slice from current offset till the end where it falls into chunk
chunk_bytes = CHUNK_SIZE - chunk_offset 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 # 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 # move offsets within buffer
@tell += chunk_data.bytesize @tell += data_slice.bytesize
@size = [size, tell].max @size = [size, tell].max
end end
...@@ -183,12 +186,12 @@ module Gitlab ...@@ -183,12 +186,12 @@ module Gitlab
current_chunk.tap do |chunk| current_chunk.tap do |chunk|
raise FailedToGetChunkError unless chunk raise FailedToGetChunkError unless chunk
@chunk = chunk.data @chunk_data = chunk.data
@chunk_range = chunk.range @chunk_range = chunk.range
end end
end end
@chunk[chunk_offset..CHUNK_SIZE] @chunk_data.byteslice(chunk_offset, CHUNK_SIZE)
end end
def chunk_offset def chunk_offset
......
...@@ -85,11 +85,11 @@ module Gitlab ...@@ -85,11 +85,11 @@ module Gitlab
break if data.empty? break if data.empty?
chunk_bytes = [BUFFER_SIZE - chunk_offset, length].min 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 out << data_slice
@tell += chunk_data.bytesize @tell += data_slice.bytesize
length -= chunk_data.bytesize length -= data_slice.bytesize
end end
out = out.join out = out.join
......
...@@ -116,6 +116,19 @@ describe Gitlab::Ci::Trace::ChunkedIO, :clean_gitlab_redis_cache do ...@@ -116,6 +116,19 @@ describe Gitlab::Ci::Trace::ChunkedIO, :clean_gitlab_redis_cache do
chunked_io.each_line { |line| } chunked_io.each_line { |line| }
end end
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 end
context "#read" do context "#read" do
...@@ -143,6 +156,22 @@ describe Gitlab::Ci::Trace::ChunkedIO, :clean_gitlab_redis_cache do ...@@ -143,6 +156,22 @@ describe Gitlab::Ci::Trace::ChunkedIO, :clean_gitlab_redis_cache do
end end
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 context 'when read only first 100 bytes' do
let(:length) { 100 } let(:length) { 100 }
...@@ -266,6 +295,40 @@ describe Gitlab::Ci::Trace::ChunkedIO, :clean_gitlab_redis_cache do ...@@ -266,6 +295,40 @@ describe Gitlab::Ci::Trace::ChunkedIO, :clean_gitlab_redis_cache do
expect(chunked_io.readline).to eq(string_io.readline) expect(chunked_io.readline).to eq(string_io.readline)
end end
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 end
context "#write" do context "#write" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment