Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ThreadError: can't move to the enclosed thread group #576

Closed
alexpapworth opened this issue Apr 16, 2020 · 8 comments · Fixed by #577
Closed

ThreadError: can't move to the enclosed thread group #576

alexpapworth opened this issue Apr 16, 2020 · 8 comments · Fixed by #577
Labels
Bug

Comments

@alexpapworth
Copy link

Hello. I've got an error when calling Airbrake.notify within a fork.

Received exception when reporting failure: #<ThreadError: can't move to the enclosed thread group>
/gems/ruby-2.3.8/gems/airbrake-ruby-4.14.0/lib/airbrake-ruby/thread_pool.rb:121:in `add'
/gems/ruby-2.3.8/gems/airbrake-ruby-4.14.0/lib/airbrake-ruby/thread_pool.rb:121:in `block in spawn_workers'
/gems/ruby-2.3.8/gems/airbrake-ruby-4.14.0/lib/airbrake-ruby/thread_pool.rb:121:in `times'
/gems/ruby-2.3.8/gems/airbrake-ruby-4.14.0/lib/airbrake-ruby/thread_pool.rb:121:in `spawn_workers'
/gems/ruby-2.3.8/gems/airbrake-ruby-4.14.0/lib/airbrake-ruby/thread_pool.rb:86:in `block in has_workers?'
/gems/ruby-2.3.8/gems/airbrake-ruby-4.14.0/lib/airbrake-ruby/thread_pool.rb:81:in `synchronize'
/gems/ruby-2.3.8/gems/airbrake-ruby-4.14.0/lib/airbrake-ruby/thread_pool.rb:81:in `has_workers?'
/gems/ruby-2.3.8/gems/airbrake-ruby-4.14.0/lib/airbrake-ruby/async_sender.rb:41:in `has_workers?'
/gems/ruby-2.3.8/gems/airbrake-ruby-4.14.0/lib/airbrake-ruby/notice_notifier.rb:122:in `default_sender'
/gems/ruby-2.3.8/gems/airbrake-ruby-4.14.0/lib/airbrake-ruby/notice_notifier.rb:36:in `notify'
/gems/ruby-2.3.8/gems/airbrake-ruby-4.14.0/lib/airbrake-ruby.rb:169:in `notify'

I notice a similar discussion in #312, but the @mutex.synchronize block in def has_workers? doesn't seem to be working for me.

Any ideas on how to fix this?
@kyrylo kyrylo added the Bug label Apr 17, 2020
@kyrylo
Copy link
Contributor

kyrylo commented Apr 17, 2020

Hi, thanks for the report. Can you reproduce that reliably? I have an idea for the fix but it would be really nice to confirm it to be 100% sure.

@alexpapworth
Copy link
Author

Alright, that happens as part of my test suite when running on Jenkins. It fails there 100% of the time, so I’m happy to test out some code you have to see if that works.

Unfortunately the error is embedded a few files deep, so let me see if I can extract the key components into a smaller script that also fails reliably.

@alexpapworth
Copy link
Author

Spent some time trying to extract a smaller test script, but wasn't able to.

The issue is still happening 100% of the time, so I'll be able to test any code you have against my codebase.

@kyrylo
Copy link
Contributor

kyrylo commented Apr 21, 2020

Thanks a lot! It's really helpful that you can repro that in 100% of cases. I will publish my guess as a PR and we will test it against your app.

kyrylo added a commit that referenced this issue Apr 22, 2020
@kyrylo
thread_pool: create a new thread group per process
7cf4589 
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`.
@kyrylo kyrylo mentioned this issue Apr 22, 2020
Merged
kyrylo added a commit that referenced this issue Apr 22, 2020
@kyrylo
thread_pool: create a new thread group per process
b31d9e3 
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`.
kyrylo added a commit that referenced this issue Apr 22, 2020
@kyrylo
thread_pool: create a new thread group per process
92f8ff9 
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 process. The problem is that that `ThreadGroup` is already
finalised in that parent process and no new threads can be added anymore to the
group. The fix is to create a new `ThreadGroup` on `fork`. This way every
process will have its own `ThreadGroup`.
kyrylo added a commit that referenced this issue Apr 22, 2020
@kyrylo
thread_pool: create a new thread group per process
f9054be 
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 process. The problem is that that `ThreadGroup` is already
finalised in that parent process and no new threads can be added anymore to the
group. The fix is to create a new `ThreadGroup` on `fork`. This way every
process will have its own `ThreadGroup`.
@kyrylo
Copy link
Contributor

kyrylo commented Apr 22, 2020

I managed to find a repro case and came up with #577

Can you please give it a try?

@alexpapworth
Copy link
Author

Just ran my test using the new branch three times (to be sure). No ThreadError errors!

That's a big thumbs up from me!

@kyrylo
Copy link
Contributor

kyrylo commented Apr 22, 2020

Awesome, thanks a lot!

@kyrylo
Copy link
Contributor

kyrylo commented Apr 22, 2020

I released this fix in v4.14.1.

@hirogeek hirogeek mentioned this issue Feb 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

thread_pool: create a new thread group per process airbrake/airbrake-ruby
2 participants
@kyrylo @alexpapworth