From 89f8181429a27742db259082a1f393a3be1517d5 Mon Sep 17 00:00:00 2001 From: Shizuo Fujita Date: Thu, 19 Mar 2026 15:34:50 +0900 Subject: [PATCH 1/5] Fix memory leak by ensuring rb_call_super is called in Watcher_Detach --- ext/cool.io/watcher.h | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/ext/cool.io/watcher.h b/ext/cool.io/watcher.h index b4892f2..1b96c85 100644 --- a/ext/cool.io/watcher.h +++ b/ext/cool.io/watcher.h @@ -33,13 +33,11 @@ if(watcher_data->loop == Qnil) \ rb_raise(rb_eRuntimeError, "not attached to a loop"); \ \ - if (watcher_data->enabled == 0) { \ - /* Ignore because watcher was already detached. */ \ - return Qnil; \ + if (watcher_data->enabled) { \ + loop_data = Coolio_Loop_ptr(watcher_data->loop); \ + \ + ev_##watcher_type##_stop(loop_data->ev_loop, &watcher_data->event_types.ev_##watcher_type); \ } \ - loop_data = Coolio_Loop_ptr(watcher_data->loop); \ - \ - ev_##watcher_type##_stop(loop_data->ev_loop, &watcher_data->event_types.ev_##watcher_type); \ rb_call_super(0, 0) #define Watcher_Enable(watcher_type, watcher) \ From 7a45665b087f3192a285f3394884514daa22dd50 Mon Sep 17 00:00:00 2001 From: Shizuo Fujita Date: Thu, 19 Mar 2026 16:49:50 +0900 Subject: [PATCH 2/5] Refactor Watcher_Detach to use Ruby-level disable method Instead of manually calling `ev_stop` within the C macro, delegate the disablement process to the existing Ruby-level `disable` method via `rb_funcall`. This refactoring ensures that when a watcher is detached while still enabled, all internal states are safely and correctly updated. Specifically, it guarantees that the `@active_watchers` counter is decremented (preventing potential event loop hangs) and the `enabled` flag is properly set to 0. --- ext/cool.io/watcher.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ext/cool.io/watcher.h b/ext/cool.io/watcher.h index 1b96c85..2d7b1d1 100644 --- a/ext/cool.io/watcher.h +++ b/ext/cool.io/watcher.h @@ -26,7 +26,6 @@ #define Watcher_Detach(watcher_type, watcher) \ struct Coolio_Watcher *watcher_data; \ - struct Coolio_Loop *loop_data; \ \ watcher_data = Coolio_Watcher_ptr(watcher); \ \ @@ -34,9 +33,7 @@ rb_raise(rb_eRuntimeError, "not attached to a loop"); \ \ if (watcher_data->enabled) { \ - loop_data = Coolio_Loop_ptr(watcher_data->loop); \ - \ - ev_##watcher_type##_stop(loop_data->ev_loop, &watcher_data->event_types.ev_##watcher_type); \ + rb_funcall(watcher, rb_intern("disable"), 0); \ } \ rb_call_super(0, 0) From bd27678f3e7c835eb1646b62e1880ce445e8e08c Mon Sep 17 00:00:00 2001 From: Shizuo Fujita Date: Thu, 19 Mar 2026 17:25:06 +0900 Subject: [PATCH 3/5] Disable fail-fast --- .github/workflows/test.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index f14d372..e0f4e9f 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -12,6 +12,7 @@ jobs: continue-on-error: ${{matrix.experimental}} strategy: + fail-fast: false matrix: os: - ubuntu From b9d1416f99e93834bbde43979d9e3b5327f272b0 Mon Sep 17 00:00:00 2001 From: Shizuo Fujita Date: Thu, 19 Mar 2026 18:01:46 +0900 Subject: [PATCH 4/5] Protect ev_stop with an enabled check in Watcher_Detach macro On macOS (kqueue), calling `ev_stop` multiple times on the same watcher or after it has been partially decommissioned can trigger a Segmentation Fault. By checking `watcher_data->enabled` before invoking the C-level stop function, we prevent these race conditions and reentrancy issues. --- ext/cool.io/watcher.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/cool.io/watcher.h b/ext/cool.io/watcher.h index 2d7b1d1..f795d28 100644 --- a/ext/cool.io/watcher.h +++ b/ext/cool.io/watcher.h @@ -61,10 +61,10 @@ if(watcher_data->loop == Qnil) \ rb_raise(rb_eRuntimeError, "not attached to a loop"); \ \ - rb_call_super(0, 0); \ - \ - loop_data = Coolio_Loop_ptr(watcher_data->loop); \ - \ - ev_##watcher_type##_stop(loop_data->ev_loop, &watcher_data->event_types.ev_##watcher_type) + if (watcher_data->enabled) { \ + loop_data = Coolio_Loop_ptr(watcher_data->loop); \ + ev_##watcher_type##_stop(loop_data->ev_loop, &watcher_data->event_types.ev_##watcher_type); \ + } \ + rb_call_super(0, 0); #endif From 7195b2ed9c225b540522fa0d0a0d797dbecc200a Mon Sep 17 00:00:00 2001 From: Shizuo Fujita Date: Fri, 20 Mar 2026 16:57:37 +0900 Subject: [PATCH 5/5] Add test --- spec/detach_race_condition_spec.rb | 45 ++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/spec/detach_race_condition_spec.rb b/spec/detach_race_condition_spec.rb index b22970c..5d11be2 100644 --- a/spec/detach_race_condition_spec.rb +++ b/spec/detach_race_condition_spec.rb @@ -47,4 +47,49 @@ def on_readable end }.not_to raise_error end + + class HttpHandler < Coolio::IO + RESPONSE = "HTTP/1.1 200 OK\r\nContent-Length: 1024\r\nConnection: close\r\n\r\n" + ("X" * 1024) + + def on_connect + end + + def on_read(data) + write(RESPONSE) + end + + def on_write_complete + close + end + end + + # https://github.com/socketry/cool.io/issues/89 + it "does not cause memory leaks" do + port = 18989 + loop = Coolio::Loop.default + + server = Coolio::TCPServer.new('127.0.0.1', port, HttpHandler) + server.attach(loop) + + event_thread = Thread.new { loop.run } + + request = "GET / HTTP/1.1\r\nHost: localhost\r\n\r\n" + + 10.times do |iteration| + begin + sock = TCPSocket.new('127.0.0.1', port) + sock.write(request) + sock.read + sock.close + rescue => e + sleep 0.01 + retry + end + end + + server.close + event_thread.join + + expect(loop.watchers).to be_empty + end end