Skip to content

Commit 3ed7da2

Browse files
committed
Fix CI failures across Ruby versions and platforms
Three separate failures: Ruby 2.6 and earlier have no rb_gc_mark_movable, rb_gc_location, or dcompact field, so closure.c did not compile. Detect rb_gc_mark_movable in extconf. Without it there is no compaction either, so plain marking holds the reference in place and the compact callback can be omitted. Ruby 3.1 and earlier spell GC.verify_compaction_references's expand_heap: keyword as double_heap:, so the new test raised ArgumentError there. Rescue and retry with the old spelling, and treat a Ruby without compaction support as nothing to provoke. expand_heap: doubles the heap, and the pages stay. That changes how often the collector runs for the rest of the suite, and test_no_memory_leak read the extra growth inside its measured block as a leak on Linux. Run the compaction scenario in a separate process, so the expansion dies with it. The subprocess still reports the fault: the child dies with SIGSEGV when closure.c stores the closure address without the mark and compact callbacks.
1 parent f695b26 commit 3ed7da2

3 files changed

Lines changed: 49 additions & 15 deletions

File tree

‎ext/fiddle/closure.c‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,12 @@ closure_memsize(const void * ptr)
5555
return size;
5656
}
5757

58+
/* Ruby 2.6 and earlier have no compaction, so marking the reference plainly
59+
* holds it in place there. */
60+
#ifndef HAVE_RB_GC_MARK_MOVABLE
61+
# define rb_gc_mark_movable rb_gc_mark
62+
#endif
63+
5864
/* Fiddle::Closure#free and initialize_rescue set the data pointer to NULL.
5965
* The garbage collector does not call these functions with a NULL pointer,
6066
* but this file is the only one in the extension that clears the pointer,
@@ -68,6 +74,7 @@ closure_mark(void *ptr)
6874
}
6975
}
7076

77+
#ifdef HAVE_RB_GC_MARK_MOVABLE
7178
static void
7279
closure_compact(void *ptr)
7380
{
@@ -76,14 +83,17 @@ closure_compact(void *ptr)
7683
closure->self = rb_gc_location(closure->self);
7784
}
7885
}
86+
#endif
7987

8088
const rb_data_type_t closure_data_type = {
8189
.wrap_struct_name = "fiddle/closure",
8290
.function = {
8391
.dmark = closure_mark,
8492
.dfree = dealloc,
8593
.dsize = closure_memsize,
86-
.dcompact = closure_compact
94+
#ifdef HAVE_RB_GC_MARK_MOVABLE
95+
.dcompact = closure_compact,
96+
#endif
8797
},
8898
.flags = FIDDLE_DEFAULT_TYPED_DATA_FLAGS,
8999
};

‎ext/fiddle/extconf.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ def enable_debug_build_flag(flags)
241241
end
242242

243243
have_func("rb_str_to_interned_str")
244+
have_func("rb_gc_mark_movable") # RUBY_VERSION >= 2.7
244245
have_const("RUBY_TYPED_EMBEDDABLE", "ruby.h") # RUBY_VERSION >= 3.3
245246
create_makefile 'fiddle' do |conf|
246247
if !libffi

‎test/fiddle/test_closure.rb‎

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -177,24 +177,47 @@ def test_call_after_compaction
177177
omit("Need GC.verify_compaction_references")
178178
end
179179
omit("Need CRuby") unless RUBY_ENGINE == "ruby"
180+
require "envutil" unless defined?(EnvUtil)
180181

181-
closure_class = Class.new(Closure) do
182-
def call(a, b)
183-
a + b
182+
# A separate process, because expand_heap: doubles the heap and the pages
183+
# stay. Expanding this process's heap changes how often the collector runs
184+
# afterwards, and test_no_memory_leak reads that as growth.
185+
script = <<~'RUBY'
186+
require "fiddle"
187+
188+
closure_class = Class.new(Fiddle::Closure) do
189+
def call(a, b)
190+
a + b
191+
end
184192
end
185-
end
186193
187-
# The closure must be reachable only through the heap. A local variable is
188-
# pinned by the conservative machine-stack scan. A pinned closure does not
189-
# move, and then this test cannot fail.
190-
holder = [closure_class.new(TYPE_INT, [TYPE_INT, TYPE_INT])]
191-
begin
192-
GC.verify_compaction_references(expand_heap: true, toward: :empty)
193-
func = Function.new(holder[0].to_i, [TYPE_INT, TYPE_INT], TYPE_INT)
194-
assert_equal(42, func.call(40, 2))
195-
ensure
194+
# The closure must be reachable only through the heap. A local variable is
195+
# pinned by the conservative machine-stack scan. A pinned closure does not
196+
# move, and then this test cannot fail.
197+
holder = [closure_class.new(Fiddle::TYPE_INT,
198+
[Fiddle::TYPE_INT, Fiddle::TYPE_INT])]
199+
begin
200+
begin
201+
GC.verify_compaction_references(expand_heap: true, toward: :empty)
202+
rescue ArgumentError
203+
# Ruby 3.1 and earlier spell expand_heap: as double_heap:
204+
GC.verify_compaction_references(double_heap: true, toward: :empty)
205+
end
206+
rescue NotImplementedError
207+
# A Ruby without compaction cannot move the closure. The call below
208+
# still exercises the plain path.
209+
end
210+
func = Fiddle::Function.new(holder[0].to_i,
211+
[Fiddle::TYPE_INT, Fiddle::TYPE_INT],
212+
Fiddle::TYPE_INT)
213+
puts(func.call(40, 2))
196214
holder[0].free
197-
end
215+
RUBY
216+
load_path_args = $LOAD_PATH.flat_map {|path| ["-I", path]}
217+
stdout, stderr, status = EnvUtil.invoke_ruby([*load_path_args, "-e", script],
218+
"", true, true)
219+
assert(status.success?, stderr)
220+
assert_equal("42", stdout.chomp)
198221
end
199222

200223
def test_ractor_shareable

0 commit comments

Comments
 (0)