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.

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

Make workers by default not call parent's at_exit handlers at exit #230

Open
dpsi opened this issue Nov 14, 2018 · 5 comments
Open

Make workers by default not call parent's at_exit handlers at exit #230

dpsi opened this issue Nov 14, 2018 · 5 comments

Comments

@dpsi
Copy link

dpsi commented Nov 14, 2018

Cucumber unfortunately utilizes the at_exit method as hook. The Parallel gem by default uses the Kernel#fork method which means that when the forked process exits, it will call the parent's at_exit handler. I am not sure if this is intended behaviour for the Parallel gem, but it does cause some unexpected bugs to occur in cucumber, or any other program that uses at_exit in a similiar way. Capybara, minitest, are some others I think use at_exit like this.

Here is the issue I opened with cucumber: cucumber/cucumber-ruby#1327

I was wondering if there was any interest in making Thread the default, or perhaps changing how the #fork workers exit. I think calling exit! will make it not call the parents at_exit handlers, but calling that from inside the Parallel block (in user code) ended up with Deadworker exception which makes sense after perusing the code.

PS: I love how this gem is essentially 1 file, very nice concise code

@grosser
Copy link
Owner

grosser commented Nov 14, 2018 via email

@dpsi
Copy link
Author

dpsi commented Nov 14, 2018

I don't suppose there is a way to globally configure Parallel to default to threads instead of Kernel#fork? Reason being is that I don't have control over how others use Parallel so while my own code would be cucumber-safe, others would not.

@grosser
Copy link
Owner

grosser commented Nov 14, 2018 via email

@dpsi
Copy link
Author

dpsi commented Nov 15, 2018

After some more reading I see why fork is the default, which also led me to conclude that using Thread for my usecase also defeats the purpose of splitting up the work load. (Thanks GIL!)

Anyways, in a future version, having that option not to call the parent at_exit handlers would be nice to have.

@grosser
Copy link
Owner

grosser commented Nov 15, 2018 via email

Hamms added a commit to code-dot-org/code-dot-org that referenced this issue Jun 1, 2020
Because we run the sync from a script that uses the only_one_running helper, which in turn uses ruby's `at_exit` hook, which doesn't play nice with forked processes: grosser/parallel#230

Threads also work better for our use case anyway.
Hamms added a commit to code-dot-org/code-dot-org that referenced this issue Jun 1, 2020
Because apparently worker processes will call their parent's `at_exit` when they themselves exit. See grosser/parallel#230
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants