Skip to content

Commit

Permalink
thread_pool: create a new thread group per process
Browse files Browse the repository at this point in the history
Fixes #576 (ThreadError: can't move to the enclosed thread group)

When we `Airbrake.notify` inside a `fork` block, `ThreadPool` tries to respawn
workers (which were killed by the `fork`) to the same `ThreadGroup`, which was
created in the parent thread. The problem is that that `ThreadGroup` is already
finalised in that parent thread and no new threads can be added anymore.  The
fix is to create a new `ThreadGroup` on `fork`. This way every process will have
its own `ThreadGroup`.
  • Loading branch information
kyrylo committed Apr 22, 2020
1 parent 1be7b15 commit b31d9e3
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ Airbrake Ruby Changelog

### master

* Fixed `ThreadError: can't move to the enclosed thread group` when using
`notify` inside a `fork`
([#577](https://github.com/airbrake/airbrake-ruby/pull/577))

### [v4.14.0][v4.14.0] (April 10, 2020)

* Fixed a bug where some default filters are not appended
Expand Down
1 change: 1 addition & 0 deletions lib/airbrake-ruby/thread_pool.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ def has_workers?

if @pid != Process.pid && @workers.list.empty?
@pid = Process.pid
@workers = ThreadGroup.new
spawn_workers
end

Expand Down
17 changes: 16 additions & 1 deletion spec/thread_pool_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,27 @@
expect(subject).not_to have_workers
end

it "respawns workers on fork()", skip: %w[jruby].include?(RUBY_ENGINE) do
it "respawns workers on fork()" do
skip('fork() is unsupported on JRuby') if %w[jruby].include?(RUBY_ENGINE)

pid = fork { expect(subject).to have_workers }
Process.wait(pid)
subject.close

expect(Process.last_status).to be_success
expect(subject).not_to have_workers
end

it "ensures that a new thread group is created per process" do
skip('fork() is unsupported on JRuby') if %w[jruby].include?(RUBY_ENGINE)

subject << 1
pid = fork { subject.has_workers? }
Process.wait(pid)
subject.close

expect(Process.last_status).to be_success
end
end

describe "#close" do
Expand Down

0 comments on commit b31d9e3

Please sign in to comment.