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

Fix to wakeup threads to quit loop in threads #1264

Merged
merged 3 commits into from
Oct 7, 2016

Conversation

tagomoris
Copy link
Member

It's done before returning from stop/after_shutdown, but after setting flag to break while-loop.
This change suppresses log messages like 'thread doesn't exit correctly (killed or other reason)' in shutdown sequence.

It's done before returning from stop/after_shutdown, but after setting flag to break while-loop.
This change suppresses log messages like 'thread doesn't exit correctly (killed or other reason)' in shutdown sequence.
@tagomoris
Copy link
Member Author

@repeatedly could you review this change?

@@ -416,6 +420,12 @@ def before_shutdown
force_flush
end
@buffer.before_shutdown
# Need to ensure to stop enqueueing ... after #shutdown, we cannot write any data
@output_enqueue_thread_running = false
if @output_enqueue_thread && @output_enqueue_thread.alive?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to check status?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to do so. Living threads are running or sleeping.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see

@_threads_mutex.synchronize do
@_threads.each_pair do |obj_id, thread|
@_threads.values.each do |thread|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

each_value is better for avoding temporary object allocation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed to do so.

super
wakeup_threads = []
@_threads_mutex.synchronize do
@_threads.values.each do |thread|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@repeatedly
Copy link
Member

Others are okay for me.

@tagomoris
Copy link
Member Author

Okay. I pushed to use each_value instead of values.each.
I'll merge this after CI green.

@tagomoris tagomoris merged commit ca20c54 into master Oct 7, 2016
@tagomoris tagomoris deleted the wakeup-threads-in-shutdown-sequence-to-quit-sleep branch October 7, 2016 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants