Skip to content

Commit de042d7

Browse files
committed
fix: release wake-up connections when shutdown fails
Cleanup ran after component shutdown, so a component raising in stop skipped it and left listener connections open. Move the release into an ensure so a failed shutdown cannot leak connections held outside the pool.
1 parent e93724c commit de042d7

2 files changed

Lines changed: 36 additions & 6 deletions

File tree

lib/solid_objects/supervisor.rb

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,17 @@ def start
4444
def stop
4545
return unless @started
4646

47-
components.each(&:request_shutdown)
48-
join_until_timeout
49-
components.reject(&:stopped?).each(&:stop)
50-
release_wake_up
51-
@started = false
52-
SolidObjects.instrument(:"supervisor.stopped", component_count: components.length)
47+
begin
48+
components.each(&:request_shutdown)
49+
join_until_timeout
50+
components.reject(&:stopped?).each(&:stop)
51+
ensure
52+
# Connections held outside the pool must be released even when a
53+
# component fails to stop, or they accumulate across restarts.
54+
release_wake_up
55+
@started = false
56+
SolidObjects.instrument(:"supervisor.stopped", component_count: components.length)
57+
end
5358
end
5459

5560
private

test/integration/postgresql_wake_up_test.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,31 @@ class PostgresqlWakeUpTest < ActiveSupport::TestCase
103103
SolidObjects.instance_variable_set(:@wake_up, nil)
104104
end
105105

106+
test "a failing component shutdown still releases listener connections" do
107+
SolidObjects.configuration.wake_up_adapter = @adapter
108+
SolidObjects.instance_variable_set(:@wake_up, nil)
109+
@adapter.listen
110+
supervisor = SolidObjects::Supervisor.new(
111+
worker_count: 1,
112+
effect_worker_count: 0,
113+
broadcast_worker_count: 0,
114+
reminder_scheduler_count: 0
115+
)
116+
supervisor.start
117+
supervisor.send(:components).each do |component|
118+
component.define_singleton_method(:stop) { raise "boom" }
119+
component.define_singleton_method(:stopped?) { false }
120+
end
121+
122+
assert_raises(RuntimeError) { supervisor.stop }
123+
124+
refute @adapter.send(:connections).any?,
125+
"a failed shutdown must still release connections opened outside the pool"
126+
ensure
127+
SolidObjects.configuration.wake_up_adapter = nil
128+
SolidObjects.instance_variable_set(:@wake_up, nil)
129+
end
130+
106131
test "the helper selects notifications on PostgreSQL" do
107132
assert_instance_of(
108133
SolidObjects::WakeUpAdapters::Postgresql,

0 commit comments

Comments
 (0)