Skip to content

Commit

Permalink
merge revision(s) 548086b,3dc8cde70078ccb38f5f4b0818ad5eecded01bd5,e0…
Browse files Browse the repository at this point in the history
…cf80d666d4b5df3229f030a16d10d21323508e: [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(-)
  • Loading branch information
nagachika committed Mar 25, 2023
1 parent bdbe605 commit cd4a997
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 5 deletions.
43 changes: 39 additions & 4 deletions gc.c
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}
Expand Down
18 changes: 18 additions & 0 deletions test/ruby/test_weakmap.rb
Expand Up @@ -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
2 changes: 1 addition & 1 deletion version.h
Expand Up @@ -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
Expand Down

0 comments on commit cd4a997

Please sign in to comment.