Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Commit

Permalink
Merge #7202
Browse files Browse the repository at this point in the history
7202: Fix thread leak in compact index specs r=deivid-rodriguez a=deivid-rodriguez

### What was the end-user problem that led to this PR?

The problem was that sometimes CI fails randomly. For example, https://travis-ci.org/bundler/bundler/jobs/544642885.

### What was your diagnosis of the problem?

My diagnosis was that the spec after it was mocking `Worker#stop` and thus preventing the threads to be actually stopped. That leak would after the spec checking for the number of threads.

### What is your fix for the problem, implemented in this PR?

My fix is to also call the original `#stop` method, so that no threads are left behind.

### Why did you choose this fix out of the possible options?

There's an alternative option, which is to completely remove this spec. In my opinion, this spec is checking exactly the same thing as the previous spec (the whole purpose of `Worker#stop` is to stop threads), but in a more brittle way. So I'm tempted to completely kill the spec instead.


Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
  • Loading branch information
bundlerbot and deivid-rodriguez committed Jun 17, 2019
2 parents 0d41790 + 81ad85b commit 4dc6d1d
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion spec/bundler/fetcher/compact_index_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
end

it "calls worker#stop during the run" do
expect_any_instance_of(Bundler::Worker).to receive(:stop).at_least(:once)
expect_any_instance_of(Bundler::Worker).to receive(:stop).at_least(:once).and_call_original

compact_index.specs_for_names(["lskdjf"])
end
Expand Down

0 comments on commit 4dc6d1d

Please sign in to comment.