diff options
| author | Jacob Vosmaer <jacob@gitlab.com> | 2019-01-18 13:05:45 +0300 |
|---|---|---|
| committer | Zeger-Jan van de Weg <git@zjvandeweg.nl> | 2019-01-18 13:05:45 +0300 |
| commit | 06228ac1db97007c1b1ff06a1fb9e231642dc7a8 (patch) | |
| tree | d7dcfa90149dbc4dea8bc71a585318336ce0849a | |
| parent | cb94c2b97427357f30678ce7164bb51250546d93 (diff) | |
Ensure that we kill ruby Gitlab::Git::Popen reader threads
| -rw-r--r-- | changelogs/unreleased/ruby-thread-exception.yml | 5 | ||||
| -rw-r--r-- | ruby/lib/gitlab/git/popen.rb | 37 |
2 files changed, 30 insertions, 12 deletions
diff --git a/changelogs/unreleased/ruby-thread-exception.yml b/changelogs/unreleased/ruby-thread-exception.yml new file mode 100644 index 000000000..5c60865b9 --- /dev/null +++ b/changelogs/unreleased/ruby-thread-exception.yml @@ -0,0 +1,5 @@ +--- +title: Ensure that we kill ruby Gitlab::Git::Popen reader threads +merge_request: 1040 +author: +type: fixed diff --git a/ruby/lib/gitlab/git/popen.rb b/ruby/lib/gitlab/git/popen.rb index 5fb595cef..6d4ef34d9 100644 --- a/ruby/lib/gitlab/git/popen.rb +++ b/ruby/lib/gitlab/git/popen.rb @@ -22,19 +22,26 @@ module Gitlab # Mimic what Ruby does with capture3: https://github.com/ruby/ruby/blob/1ec544695fa02d714180ef9c34e755027b6a2103/lib/open3.rb#L257-L273 err_reader = Thread.new { stderr.read } - yield(stdin) if block_given? - stdin.close - - if lazy_block - cmd_output = lazy_block.call(stdout.lazy) - cmd_status = 0 - break - else - cmd_output << stdout.read + begin + yield(stdin) if block_given? + stdin.close + + if lazy_block + cmd_output = lazy_block.call(stdout.lazy) + cmd_status = 0 + break + else + cmd_output << stdout.read + end + + cmd_output << err_reader.value if include_stderr + cmd_status = wait_thr.value.exitstatus + ensure + # When Popen3.open3 returns, the stderr reader gets closed, which causes + # an exception in the err_reader thread. Kill the thread before + # returning from Popen3.open3. + err_reader.kill end - - cmd_output << err_reader.value if include_stderr - cmd_status = wait_thr.value.exitstatus end [cmd_output, cmd_status] @@ -76,7 +83,13 @@ module Gitlab wout.close unless wout.closed? werr.close unless werr.closed? + # rout is shared with out_reader. To prevent an exception in that + # thread, kill the thread before closing rout. The same goes for rerr + # below. + out_reader.kill rout.close + + err_reader.kill rerr.close end end |