Skip to content

Commit

Permalink
Do not enclose the worker ThreadGroup (#714)
Browse files Browse the repository at this point in the history
Enclosing the ThreadGroup has caused issues for anyone using
version 0.3.1 of the timeout gem. @thompiler and I were able to
replicate the issue by running a rails 7.0.4 app with ruby version 3.2.1
and the timeout gem at version 0.3.1 `timeout, '= 0.3.1'` in the
Gemfile. It was also important to run the rails server with
`WEB_CONCURRENCY=3`.

```
WEB_CONCURRENCY=3 bin/rails s
```
Then watch the airbrake logs you may need to enable this in your
airbrake.rb initializer. Any Net::HTTP request will trigger a timeout on
the enclosed ThreadGroup and cause an issue. This could be sending an
Airbrake notice, performance data or fetching the remote config.
  • Loading branch information
mmcdaris authored Mar 22, 2023
1 parent c08aa50 commit b05ce07
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 4 deletions.
1 change: 0 additions & 1 deletion lib/airbrake-ruby/thread_pool.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ def closed?

def spawn_workers
@worker_size.times { @workers.add(spawn_worker) }
@workers.enclose
end

private
Expand Down
10 changes: 7 additions & 3 deletions spec/thread_pool_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -180,17 +180,21 @@
describe "#spawn_workers" do
after { thread_pool.close }

it "spawns an enclosed thread group" do
let(:worker_size) { 3 }

# We avoid enclosed thread groups since they cause issues for anyone using timeout 0.3.1
# More info: https://github.com/airbrake/airbrake-ruby/issues/713
it "spawns an unenclosed thread group" do
expect(thread_pool.workers).to be_a(ThreadGroup)
expect(thread_pool.workers).to be_enclosed
expect(thread_pool.workers).not_to be_enclosed
end

it "spawns threads that are alive" do
expect(thread_pool.workers.list).to all(be_alive)
end

it "spawns exactly `workers_size` workers" do
expect(thread_pool.workers.list.size).to eq(worker_size)
expect(thread_pool.workers.list.size).to eq(3)
end
end
end

0 comments on commit b05ce07

Please sign in to comment.