Bug #22384
open
Regexp compile error in the 2nd+ branch of a top-level alternation leaks the parse tree (regression from #13332, Ruby 4.0)
Description
Affects: ruby 4.0.6 and 4.0.7 (official docker images and a jemalloc-linked build), master (commit still present). Not affected: 3.4.6.
Repro (plain Ruby, no extensions):
def rss_kb = File.read('/proc/self/status')[/VmRSS:\s+(\d+)/, 1].to_i
GC.start; b = rss_kb
500_000.times { Regexp.new("a|b(c") rescue nil } # RegexpError in the 2nd branch
GC.start; puts "#{rss_kb - b} KB" # 4.0.6/4.0.7: ~65500 KB (134 B per compile); 3.4.6: 0
500_000.times { Regexp.new("b(c") rescue nil } # same error, no alternation: 0 KB on both
Also leaks: "a|b|c(d" (3rd branch), "a|b)c(d". Does not leak: nested "(a|b(c)".
Why it matters: Regexp#to_s (rb_reg_str_with_term, re.c) re-runs onig_new on the inner pattern of any regexp whose source starts with "(?flags:" and ends with ")" to decide whether the prefix can be dropped. For every regexp shaped /(?:a|b)c(d)/ that inner pattern is "a|b)c(d", which hits this leak. Rails calls Regexp#to_s on every Regexp in config.filter_parameters for every request (ActiveSupport::ParameterFilter compile_filters!, one ParameterFilter per request), and Rails precompiles filter_parameters into exactly that shape, so a Rails 7.1+/8.x app with a Regexp filter leaks ~128 B per request per process on Ruby 4.0.
Observed on production: +3.4 MB/h per puma worker, 145 MB per worker after 40 h (jemalloc profile stacks:
rb_reg_str_with_term → onig_new → onig_parse_make_tree → parse_subexp → parse_branch → parse_exp).
Cause: commit 35000ac2ed "Prevent double free for too big repetition quantifiers (#13332)", regparse.c parse_subexp(). Before it, the alternation list was assigned to *top immediately and the loop's error paths called onig_node_free(node). After it, the list lives in the local topnode and both error paths call onig_node_free(topnode) only. That is right for the fetch_token failure (node was already linked into topnode by the previous iteration), but wrong for the parse_branch failure: parse_branch returns the partially built branch in *top (see its *top = node_new_list(node, NULL) before the loop) and that node is not yet linked into topnode, so it is never freed.
Fix:
r = parse_branch(&node, tok, term, src, end, env);
if (r < 0) {
+ onig_node_free(node);
onig_node_free(topnode);
return r;
}
(parse_branch's own error path frees only the element that failed and leaves the list in *top, as it did before #13332; the top-level if (r < 0) { onig_node_free(node); return r; } a few lines above already does the same for the first branch.)
Updated by