From cd4a997e53a02e5a8b24e38a10ba5525c176980e Mon Sep 17 00:00:00 2001 From: nagachika Date: Sat, 25 Mar 2023 15:03:14 +0900 Subject: [PATCH] merge revision(s) 548086b34e3dd125edabf5dc1e46b891fad3ea9c,3dc8cde70078ccb38f5f4b0818ad5eecded01bd5,e0cf80d666d4b5df3229f030a16d10d21323508e: [Backport #19529] ObjectSpace::WeakMap: fix compaction support [Bug #19529] `rb_gc_update_tbl_refs` can't be used on `w->obj2wmap` because it's not a `VALUE -> VALUE` table, but a `VALUE -> VALUE *` table, so we need some dedicated iterator. --- test/ruby/test_weakmap.rb | 8 ++++++++ weakmap.c | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) Fix crash during compaction [Bug #19529] The fix for [Bug #19529] in commit 548086b contained a bug that crashes on the following script: ``` wm = ObjectSpace::WeakMap.new obj = Object.new 100.times do wm[Object.new] = obj GC.start end GC.compact ``` --- test/ruby/test_weakmap.rb | 10 ++++++++++ weakmap.c | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) Fix incorrect size of WeakMap buffer In wmap_final_func, j is the number of elements + 1 (since j also includes the length at the 0th index), so we should resize the buffer to size j and the new length is j - 1. --- weakmap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- gc.c | 43 +++++++++++++++++++++++++++++++++++---- test/ruby/test_weakmap.rb | 18 ++++++++++++++++ version.h | 2 +- 3 files changed, 58 insertions(+), 5 deletions(-) diff --git a/gc.c b/gc.c index 9471a953544860..6a81a5e4b3c495 100644 --- a/gc.c +++ b/gc.c @@ -11954,12 +11954,47 @@ wmap_mark_map(st_data_t key, st_data_t val, st_data_t arg) } #endif +static int +wmap_replace_ref(st_data_t *key, st_data_t *value, st_data_t _argp, int existing) +{ + *key = rb_gc_location((VALUE)*key); + + VALUE *values = (VALUE *)*value; + VALUE size = values[0]; + + for (VALUE index = 1; index <= size; index++) { + values[index] = rb_gc_location(values[index]); + } + + return ST_CONTINUE; +} + +static int +wmap_foreach_replace(st_data_t key, st_data_t value, st_data_t _argp, int error) +{ + if (rb_gc_location((VALUE)key) != (VALUE)key) { + return ST_REPLACE; + } + + VALUE *values = (VALUE *)value; + VALUE size = values[0]; + + for (VALUE index = 1; index <= size; index++) { + VALUE val = values[index]; + if (rb_gc_location(val) != val) { + return ST_REPLACE; + } + } + + return ST_CONTINUE; +} + static void wmap_compact(void *ptr) { struct weakmap *w = ptr; if (w->wmap2obj) rb_gc_update_tbl_refs(w->wmap2obj); - if (w->obj2wmap) rb_gc_update_tbl_refs(w->obj2wmap); + if (w->obj2wmap) st_foreach_with_replace(w->obj2wmap, wmap_foreach_replace, wmap_replace_ref, (st_data_t)NULL); w->final = rb_gc_location(w->final); } @@ -12072,9 +12107,9 @@ wmap_final_func(st_data_t *key, st_data_t *value, st_data_t arg, int existing) return ST_DELETE; } if (j < i) { - SIZED_REALLOC_N(ptr, VALUE, j + 1, i); - ptr[0] = j; - *value = (st_data_t)ptr; + SIZED_REALLOC_N(ptr, VALUE, j, i); + ptr[0] = j - 1; + *value = (st_data_t)ptr; } return ST_CONTINUE; } diff --git a/test/ruby/test_weakmap.rb b/test/ruby/test_weakmap.rb index c482f697c3eaaf..5ca6b0fbddacac 100644 --- a/test/ruby/test_weakmap.rb +++ b/test/ruby/test_weakmap.rb @@ -176,4 +176,22 @@ def test_no_memory_leak end end; end + + def test_compaction_bug_19529 + obj = Object.new + 100.times do |i| + GC.compact + @wm[i] = obj + end + + assert_separately(%w(--disable-gems), <<-'end;') + wm = ObjectSpace::WeakMap.new + obj = Object.new + 100.times do + wm[Object.new] = obj + GC.start + end + GC.compact + end; + end end diff --git a/version.h b/version.h index 9bcaa82d6266da..114ab901958de2 100644 --- a/version.h +++ b/version.h @@ -11,7 +11,7 @@ # define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR #define RUBY_VERSION_TEENY 4 #define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR -#define RUBY_PATCHLEVEL 217 +#define RUBY_PATCHLEVEL 218 #define RUBY_RELEASE_YEAR 2023 #define RUBY_RELEASE_MONTH 3