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
Remove Listen::Internals::ThreadPool #480
Conversation
The `@listeners` variable holds a reference to each `Listener` instance created by `Listen.to` and can only be cleared by calling `Listen.stop`. This can prevent `Listener` instances from being garbage collected if they are abandoned before `Listen.stop` is called. This commit wraps `Listener` instances in `WeakRef` before adding them to `@listeners`, to allow them to be garbage collected. Partially addresses guard#476.
# You can't kill a thread that is doing a sysread in JRuby, so | ||
# skip `join` and pray thread dies fast... | ||
if RUBY_ENGINE == 'jruby' | ||
@run_thread.kill if (@run_thread ||= nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/ParenthesesAroundCondition: Don't use parentheses around the condition of an if.
@@ -99,7 +97,15 @@ def _dir_event?(event) | |||
end | |||
|
|||
def _stop | |||
@worker && @worker.close | |||
@worker.close if (@worker ||= nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/ParenthesesAroundCondition: Don't use parentheses around the condition of an if.
@@ -74,6 +73,11 @@ def _run_worker(worker) | |||
format_string = 'fsevent: running worker failed: %s:%s called from: %s' | |||
_log_exception format_string, caller | |||
end | |||
|
|||
def _stop | |||
@worker_thread.kill.join if (@worker_thread ||= nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/ParenthesesAroundCondition: Don't use parentheses around the condition of an if.
@@ -96,6 +96,7 @@ def self.usable? | |||
private | |||
|
|||
def _stop | |||
@run_thread.kill.join if (@run_thread ||= nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/ParenthesesAroundCondition: Don't use parentheses around the condition of an if.
f9f26c2
to
5fcb42d
Compare
`Listen::Internals::ThreadPool` manages all listener threads. Thus the only way to kill and subsequently garbage collect listener threads is by calling the `Listen::Internals::ThreadPool.stop` method (typically done via `Listen.stop`). This is a problem when individual listeners must be stopped and abandoned for garbage collection. This commit removes `Listen::Internals::ThreadPool` in favor of listener instances managing their own threads. Partially addresses guard#476.
5fcb42d
to
05ac45f
Compare
I am manually rebasing and merging. |
Listen::Internals::ThreadPool
manages all listener threads. Thus the only way to kill and subsequently garbage collect listener threads is by calling theListen::Internals::ThreadPool.stop
method (typically done viaListen.stop
). This is a problem when individual listeners must be stopped and abandoned for garbage collection.This commit removes
Listen::Internals::ThreadPool
in favor of listener instances managing their own threads.Partially addresses #476.
/cc @headius
This is based on top of #477, so that should probably be merged first. Together, these PRs fix #476.