From 66349692f0d1c63c5687ee5df32548097f57f5a3 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Fri, 18 Jul 2025 10:58:41 -0400 Subject: [PATCH] Introduce free function to rb_concurrent_set_funcs If we create a key but don't insert it (due to other Ractor winning the race), then it would leak memory if we don't free it. This introduces a new function to free that memory for this case. --- concurrent_set.c | 8 ++++++++ internal/concurrent_set.h | 11 ++++------- string.c | 1 + symbol.c | 9 +++++++++ 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/concurrent_set.c b/concurrent_set.c index df93476761..2ddc3b84a6 100644 --- a/concurrent_set.c +++ b/concurrent_set.c @@ -304,6 +304,14 @@ rb_concurrent_set_find_or_insert(VALUE *set_obj_ptr, VALUE key, void *data) if (set->funcs->cmp(key, curr_key)) { // We've found a match. RB_GC_GUARD(set_obj); + + if (inserting) { + // We created key using set->funcs->create, but we didn't end + // up inserting it into the set. Free it here to prevent memory + // leaks. + if (set->funcs->free) set->funcs->free(key); + } + return curr_key; } diff --git a/internal/concurrent_set.h b/internal/concurrent_set.h index b249acd64f..76cbefab04 100644 --- a/internal/concurrent_set.h +++ b/internal/concurrent_set.h @@ -4,14 +4,11 @@ #include "ruby/atomic.h" #include "ruby/ruby.h" -typedef VALUE (*rb_concurrent_set_hash_func)(VALUE key); -typedef bool (*rb_concurrent_set_cmp_func)(VALUE a, VALUE b); -typedef VALUE (*rb_concurrent_set_create_func)(VALUE key, void *data); - struct rb_concurrent_set_funcs { - rb_concurrent_set_hash_func hash; - rb_concurrent_set_cmp_func cmp; - rb_concurrent_set_create_func create; + VALUE (*hash)(VALUE key); + bool (*cmp)(VALUE a, VALUE b); + VALUE (*create)(VALUE key, void *data); + void (*free)(VALUE key); }; VALUE rb_concurrent_set_new(const struct rb_concurrent_set_funcs *funcs, int capacity); diff --git a/string.c b/string.c index fefe05073d..1668f06e46 100644 --- a/string.c +++ b/string.c @@ -552,6 +552,7 @@ static const struct rb_concurrent_set_funcs fstring_concurrent_set_funcs = { .hash = fstring_concurrent_set_hash, .cmp = fstring_concurrent_set_cmp, .create = fstring_concurrent_set_create, + .free = NULL, }; void diff --git a/symbol.c b/symbol.c index c27aae6409..c66e7f067d 100644 --- a/symbol.c +++ b/symbol.c @@ -315,10 +315,19 @@ sym_set_create(VALUE sym, void *data) } } +static void +sym_set_free(VALUE sym) +{ + if (sym_set_sym_static_p(sym)) { + xfree(sym_set_static_sym_untag(sym)); + } +} + static const struct rb_concurrent_set_funcs sym_set_funcs = { .hash = sym_set_hash, .cmp = sym_set_cmp, .create = sym_set_create, + .free = sym_set_free, }; static VALUE