Skip to content

Commit 5bfd8e7

Browse files
committed
Fix GC compaction for compare-by-identity sets
[Bug #22064] Compare-by-identity sets use the address for hashing, so we must pin it so the object does not move in GC compaction. Objects in a compare-by-identity set is not currently pinned, causing the set to be broken if the object is moved. For example: set = Set.new.compare_by_identity o = Object.new set.add(o) puts set.include?(o) GC.verify_compaction_references(expand_heap: true, toward: :empty) puts set.include?(o) It should output true twice, but it outputs true and false.
1 parent 24a1b09 commit 5bfd8e7

2 files changed

Lines changed: 35 additions & 1 deletion

File tree

set.c

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,14 @@ struct set_object {
123123
set_table table;
124124
};
125125

126+
static int
127+
mark_and_pin_key(st_data_t key, st_data_t data)
128+
{
129+
rb_gc_mark((VALUE)key);
130+
131+
return ST_CONTINUE;
132+
}
133+
126134
static int
127135
mark_key(st_data_t key, st_data_t data)
128136
{
@@ -135,7 +143,14 @@ static void
135143
set_mark(void *ptr)
136144
{
137145
struct set_object *sobj = ptr;
138-
if (sobj->table.entries) set_table_foreach(&sobj->table, mark_key, 0);
146+
if (sobj->table.entries) {
147+
if (sobj->table.type == &identhash) {
148+
set_table_foreach(&sobj->table, mark_and_pin_key, 0);
149+
}
150+
else {
151+
set_table_foreach(&sobj->table, mark_key, 0);
152+
}
153+
}
139154
}
140155

141156
static void

test/ruby/test_set.rb

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -902,6 +902,25 @@ def test_compare_by_identity
902902
assert_equal(array.uniq.sort, set.sort)
903903
end
904904

905+
def test_compare_by_identity_compact
906+
omit "compaction is not supported on this platform" unless GC.respond_to?(:compact)
907+
908+
# [Bug #22064]
909+
assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}")
910+
begin;
911+
set = Set.new.compare_by_identity
912+
913+
o = Object.new
914+
set.add(o)
915+
916+
assert_include(set, o)
917+
918+
GC.verify_compaction_references(expand_heap: true, toward: :empty)
919+
920+
assert_include(set, o)
921+
end;
922+
end
923+
905924
def test_reset
906925
[Set, Class.new(Set)].each { |klass|
907926
a = [1, 2]

0 commit comments

Comments
 (0)