Fix dangling path pointer in StatWatcher under GC compaction

ext/cool.io/cool.io.h

@@ -55,6 +55,12 @@ struct Coolio_Watcher
   int enabled;
   VALUE loop;
 
+  /* Stable copy of the watched path for ev_stat watchers.  libev retains the
+   * path pointer passed to ev_stat_init() for the lifetime of the watcher, so
+   * it must not point into a Ruby String whose buffer GC.compact may relocate.
+   * NULL for watcher types that don't use it. */
+  char *stat_path;
+
   void (*dispatch_callback)(VALUE self, int revents);
 };
 

ext/cool.io/stat_watcher.c

@@ -79,23 +79,36 @@ void Init_coolio_stat_watcher()
  */
 static VALUE Coolio_StatWatcher_initialize(int argc, VALUE *argv, VALUE self)
 {
-	VALUE path, interval;
+  VALUE path, interval;
+  const char *path_str;
+  long path_len;
   struct Coolio_Watcher *watcher_data;
 
   rb_scan_args(argc, argv, "11", &path, &interval);
   if(interval != Qnil)
     interval = rb_convert_type(interval, T_FLOAT, "Float", "to_f");
 
   path = rb_String(path);
+  path_str = StringValueCStr(path);
   rb_iv_set(self, "@path", path);
 
   watcher_data = Coolio_Watcher_ptr(self);
 
+  /* libev keeps the path pointer passed to ev_stat_init() for the lifetime of
+   * the watcher.  RSTRING_PTR(path) points into a Ruby String whose buffer
+   * GC.compact may relocate, leaving libev with a dangling pointer.  Keep an
+   * owned copy instead; it is released in Coolio_Watcher_free(). */
+  path_len = strlen(path_str);
+  if(watcher_data->stat_path)
+    xfree(watcher_data->stat_path);
+  watcher_data->stat_path = xmalloc(path_len + 1);
+  memcpy(watcher_data->stat_path, path_str, path_len + 1);
+
   watcher_data->dispatch_callback = Coolio_StatWatcher_dispatch_callback;
   ev_stat_init(
       &watcher_data->event_types.ev_stat,
       Coolio_StatWatcher_libev_callback,
-      RSTRING_PTR(path),
+      watcher_data->stat_path,
       interval == Qnil ? 0 : NUM2DBL(interval)
   );
   watcher_data->event_types.ev_stat.data = (void *)self;

ext/cool.io/watcher.c

@@ -14,6 +14,7 @@ static VALUE cCoolio_Watcher = Qnil;
 
 static VALUE Coolio_Watcher_allocate(VALUE klass);
 static void Coolio_Watcher_mark(void *data);
+static void Coolio_Watcher_free(void *data);
 
 static VALUE Coolio_Watcher_initialize(VALUE self);
 static VALUE Coolio_Watcher_attach(VALUE self, VALUE loop);
@@ -56,7 +57,7 @@ static const rb_data_type_t Coolio_Watcher_type = {
   "Coolio::Watcher",
   {
     Coolio_Watcher_mark,
-    RUBY_DEFAULT_FREE,
+    Coolio_Watcher_free,
   },
 };
 
@@ -75,6 +76,7 @@ static VALUE Coolio_Watcher_allocate(VALUE klass)
 
   watcher_data->loop = Qnil;
   watcher_data->enabled = 0;
+  watcher_data->stat_path = NULL;
 
   return watcher;
 }
@@ -87,6 +89,16 @@ static void Coolio_Watcher_mark(void *data)
     rb_gc_mark(watcher_data->loop);
 }
 
+static void Coolio_Watcher_free(void *data)
+{
+  struct Coolio_Watcher *watcher_data = data;
+
+  if(watcher_data->stat_path)
+    xfree(watcher_data->stat_path);
+
+  xfree(watcher_data);
+}
+
 static VALUE Coolio_Watcher_initialize(VALUE self)
 {
   rb_raise(rb_eRuntimeError, "watcher base class should not be initialized directly");

spec/stat_watcher_spec.rb

@@ -18,12 +18,31 @@ def on_change(previous, current)
   end
 end
 
-def run_with_file_change(path)
+def run_with_file_change(path, compact: false)
   reactor = Cool.io::Loop.new
 
   sw = MyStatWatcher.new(path)
   sw.attach(reactor)
 
+  # libev retains the path pointer passed to ev_stat_init() for the lifetime of
+  # the watcher.  If the watcher held RSTRING_PTR(@path) directly, a compaction
+  # that relocates the @path String would leave libev dereferencing a stale
+  # address on every stat.  Force a maximal relocation here so the watcher keeps
+  # operating against a path buffer it owns rather than Ruby-managed memory.
+  if compact
+    if GC.respond_to?(:verify_compaction_references)
+      # verify_compaction_references moves every movable object to a fresh slot.
+      # Its keyword arguments have varied across Ruby versions, so fall back.
+      begin
+        GC.verify_compaction_references(expand_heap: true, toplevel: true)
+      rescue ArgumentError
+        GC.verify_compaction_references(expand_heap: true)
+      end
+    elsif GC.respond_to?(:compact)
+      GC.compact
+    end
+  end
+
   tw = Cool.io::TimerWatcher.new(INTERVAL, true)
   tw.on_timer do
     reactor.stop if sw.accessed
@@ -69,6 +88,17 @@ def delete_file(path)
     expect(watcher.previous.ino).to eq(watcher.current.ino)
   end
 
+  it "keeps firing on_change after GC compaction relocates objects" do
+    skip "GC compaction not available" unless GC.respond_to?(:compact)
+
+    watcher = run_with_file_change(TEMP_FILE_PATH, compact: true)
+    expect(watcher.accessed).to eq(true)
+  end
+
+  it "raises ArgumentError when the path contains a null byte" do
+    expect { MyStatWatcher.new("foo\0bar") }.to raise_error(ArgumentError)
+  end
+
   it "should raise when the handler does not take 2 parameters" do
     class MyStatWatcher < Cool.io::StatWatcher
       remove_method :on_change