Skip to content

Commit

Permalink
Cease holding references to listeners
Browse files Browse the repository at this point in the history
The `@listeners` variable is global state that holds a reference to each
`Listener` instance and can only be cleared by calling `Listen.stop`.
This prevents individual listeners from being garbage collected if they
are abandoned before `Listen.stop` is called.

This commit replaces `@listeners` with a comparable call to
`ObjectSpace.each_object`.

Partially addresses #476.
  • Loading branch information
jonathanhefner committed Jul 5, 2020
1 parent ba5059c commit 694e28b
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 19 deletions.
9 changes: 2 additions & 7 deletions lib/listen.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,16 @@ class << self
# @return [Listen::Listener] the listener
#
def to(*args, &block)
@listeners ||= []
Listener.new(*args, &block).tap do |listener|
@listeners << listener
end
Listener.new(*args, &block)
end

# This is used by the `listen` binary to handle Ctrl-C
#
def stop
Internals::ThreadPool.stop
@listeners ||= []

# TODO: should use a mutex for this
@listeners.each(&:stop)
@listeners = nil
ObjectSpace.each_object(Listener, &:stop)
end
end
end
6 changes: 6 additions & 0 deletions spec/lib/listen/listener_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@
allow(block).to receive(:call)
end

after do
allow(backend).to receive(:stop)
allow(processor).to receive(:teardown)
subject.stop
end

describe 'initialize' do
it { should_not be_paused }

Expand Down
16 changes: 4 additions & 12 deletions spec/lib/listen_spec.rb
Original file line number Diff line number Diff line change
@@ -1,23 +1,15 @@
RSpec.describe Listen do
let(:listener) { instance_double(Listen::Listener, stop: nil) }

after do
Listen.stop
end

describe '.to' do
it 'initalizes listener' do
expect(Listen::Listener).to receive(:new).with('/path') { listener }
it 'initializes listener' do
expect(Listen::Listener).to receive(:new).with('/path')
described_class.to('/path')
end
end

describe '.stop' do
it 'stops all listeners' do
allow(Listen::Listener).to receive(:new).with('/path') { listener }
expect(listener).to receive(:stop)
described_class.to('/path')
Listen.stop
expect(Listen::Listener.allocate).to receive(:stop)
described_class.stop
end
end
end

0 comments on commit 694e28b

Please sign in to comment.