From 3826b46dd7b1a69d70edbb4a7ecf5d2631910877 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Wed, 15 Apr 2026 03:05:32 +0300 Subject: [PATCH] cgen: fix memory leak when using closures (fixes #21019) --- vlib/builtin/closure/closure.c.v | 135 +++++++++++++++--- vlib/v/gen/c/fn.v | 47 +++++- vlib/v/markused/markused.v | 1 + .../closure_direct_call_reuses_slot_test.c.v | 47 ++++++ 4 files changed, 211 insertions(+), 19 deletions(-) create mode 100644 vlib/v/tests/fns/closure_direct_call_reuses_slot_test.c.v diff --git a/vlib/builtin/closure/closure.c.v b/vlib/builtin/closure/closure.c.v index 773591c99..ffe438dc8 100644 --- a/vlib/builtin/closure/closure.c.v +++ b/vlib/builtin/closure/closure.c.v @@ -9,6 +9,12 @@ const ppc64_architecture = int(11) type ClosureGetDataFn = fn () voidptr +struct ClosurePage { +mut: + next &ClosurePage = unsafe { nil } + exec_page_start voidptr +} + @[heap] struct Closure { ClosureMutex @@ -16,7 +22,9 @@ mut: closure_ptr voidptr closure_get_data ClosureGetDataFn = unsafe { nil } closure_cap int - v_page_size int = int(0x4000) + free_closure_ptr voidptr + pages &ClosurePage = unsafe { nil } + v_page_size int = int(0x4000) } __global g_closure = Closure{} @@ -238,6 +246,55 @@ const closure_size_1 = if 2 * u32(sizeof(voidptr)) > u32(closure_thunk.len) { } const closure_size = int(closure_size_1 & ~(u32(sizeof(voidptr)) - 1)) +@[inline] +fn closure_exec_ptr(closure voidptr) voidptr { + if is_ppc64() { + return unsafe { &u8(closure) + assumed_page_size } + } + return closure +} + +@[inline] +fn closure_return_ptr(exec_ptr voidptr) voidptr { + if is_ppc64() { + return unsafe { &u8(exec_ptr) - assumed_page_size } + } + return exec_ptr +} + +@[inline] +fn closure_slot_meta(exec_ptr voidptr) &voidptr { + return unsafe { &voidptr(&u8(exec_ptr) - assumed_page_size) } +} + +fn closure_register_page(exec_page_start voidptr) { + unsafe { + node := &ClosurePage(malloc(sizeof(ClosurePage))) + *node = ClosurePage{ + next: g_closure.pages + exec_page_start: exec_page_start + } + g_closure.pages = node + } +} + +fn closure_is_managed(exec_ptr voidptr) bool { + if isnil(exec_ptr) { + return false + } + exec_addr := unsafe { usize(exec_ptr) } + mut page := g_closure.pages + for page != unsafe { nil } { + page_addr := unsafe { usize(page.exec_page_start) } + if exec_addr >= page_addr && exec_addr < page_addr + usize(g_closure.v_page_size) { + slot_offset := exec_addr - page_addr + return slot_offset >= usize(closure_size) && slot_offset % usize(closure_size) == 0 + } + page = page.next + } + return false +} + // closure_alloc allocates executable memory pages for closures(INTERNAL COMPILER USE ONLY). fn closure_alloc() { p := closure_alloc_platform() @@ -247,6 +304,7 @@ fn closure_alloc() { // Setup executable and guard pages x := unsafe { p + g_closure.v_page_size } // End of guard page mut remaining := g_closure.v_page_size / closure_size // Calculate slot count + closure_register_page(x) g_closure.closure_ptr = x // Current allocation pointer g_closure.closure_cap = remaining // Remaining slot count @@ -306,20 +364,29 @@ fn closure_init() { fn closure_create(func voidptr, data voidptr) voidptr { closure_mtx_lock_platform() - // Handle memory exhaustion - if g_closure.closure_cap == 0 { - closure_alloc() // Allocate new memory page - } - g_closure.closure_cap-- // Decrement slot counter + mut curr_closure := g_closure.free_closure_ptr + if !isnil(curr_closure) { + unsafe { + mut p := closure_slot_meta(curr_closure) + g_closure.free_closure_ptr = p[0] + } + } else { + // Handle memory exhaustion + if g_closure.closure_cap == 0 { + closure_alloc() // Allocate new memory page + } + g_closure.closure_cap-- // Decrement slot counter - // Claim current closure slot - curr_closure := g_closure.closure_ptr + // Claim current closure slot + curr_closure = g_closure.closure_ptr + unsafe { + // Move to next available slot + g_closure.closure_ptr = &u8(g_closure.closure_ptr) + closure_size + } + } unsafe { - // Move to next available slot - g_closure.closure_ptr = &u8(g_closure.closure_ptr) + closure_size - // Write closure metadata (data + function pointer) - mut p := &voidptr(&u8(curr_closure) - assumed_page_size) + mut p := closure_slot_meta(curr_closure) if is_ppc64() { // ELFv1: guard page layout per slot: // [0] desc[0] = thunk code address <- returned as ELFv1 function pointer @@ -338,18 +405,14 @@ fn closure_create(func voidptr, data voidptr) voidptr { closure_mtx_unlock_platform() // Return executable closure object - if is_ppc64() { - // ELFv1: return descriptor address (guard page), not raw code address - return unsafe { &u8(curr_closure) - assumed_page_size } - } - return curr_closure + return closure_return_ptr(curr_closure) } // closure_data returns the userdata pointer associated with a closure object. @[direct_array_access] fn closure_data(closure voidptr) voidptr { unsafe { - mut p := &voidptr(&u8(closure) - assumed_page_size) + mut p := closure_slot_meta(closure_exec_ptr(closure)) $if ppc64 { return p[2] } $else { @@ -357,3 +420,39 @@ fn closure_data(closure voidptr) voidptr { } } } + +// closure_try_destroy frees a managed closure slot and its context when the closure is known to be temporary. +@[direct_array_access] +fn closure_try_destroy(closure voidptr) { + if isnil(closure) { + return + } + exec_ptr := closure_exec_ptr(closure) + closure_mtx_lock_platform() + if !closure_is_managed(exec_ptr) { + closure_mtx_unlock_platform() + return + } + unsafe { + mut p := closure_slot_meta(exec_ptr) + mut data := nil + if is_ppc64() { + data = p[2] + } else { + data = p[0] + } + if !isnil(data) { + free(data) + } + p[0] = g_closure.free_closure_ptr + if is_ppc64() { + p[1] = nil + p[2] = nil + p[3] = nil + } else { + p[1] = nil + } + g_closure.free_closure_ptr = exec_ptr + } + closure_mtx_unlock_platform() +} diff --git a/vlib/v/gen/c/fn.v b/vlib/v/gen/c/fn.v index a53497fef..9e2ac92e1 100644 --- a/vlib/v/gen/c/fn.v +++ b/vlib/v/gen/c/fn.v @@ -1746,6 +1746,8 @@ fn (mut g Gen) call_expr(node ast.CallExpr) { // NOTE: everything could be done this way // see my comment in parser near anon_fn mut tmp_anon_fn_var := '' + mut tmp_fn_result_var := '' + mut needs_tmp_fn_result_cleanup := false if node.left is ast.AnonFn { if node.left.inherited_vars.len > 0 { tmp_anon_fn_var = g.new_tmp_var() @@ -1806,7 +1808,27 @@ fn (mut g Gen) call_expr(node ast.CallExpr) { } } else if !g.inside_curry_call && node.left is ast.CallExpr && node.name == '' { if node.or_block.kind == .absent { - g.expr(ast.Expr(node.left)) + left_return_typ := g.recheck_concrete_type(g.unwrap_generic(node.left.return_type)) + if g.table.used_features.anon_fn && g.table.final_sym(left_return_typ).kind == .function { + tmp_fn_result_var = g.new_tmp_var() + fn_sym := g.table.final_sym(left_return_typ).info as ast.FnType + fn_type := g.fn_var_signature(ast.void_type, fn_sym.func.return_type, + fn_sym.func.params.map(it.typ), tmp_fn_result_var) + line := g.go_before_last_stmt().trim_space() + g.empty_line = true + g.write('${fn_type} = ') + g.expr(ast.Expr(node.left)) + g.writeln(';') + g.set_current_pos_as_last_stmt_pos() + g.write(line) + if g.out.last_n(1) != '\n' { + g.writeln('') + } + g.write(tmp_fn_result_var) + needs_tmp_fn_result_cleanup = true + } else { + g.expr(ast.Expr(node.left)) + } } else { ret_typ := node.return_type @@ -1889,6 +1911,18 @@ fn (mut g Gen) call_expr(node ast.CallExpr) { } else { '' } + mut tmp_fn_result_call_value := '' + mut tmp_fn_result_call_line := '' + if needs_tmp_fn_result_cleanup && !gen_or && !gen_keep_alive && !g.inside_curry_call + && node.return_type != 0 && node.return_type != ast.void_type { + tmp_fn_result_call_line = g.go_before_last_stmt() + if tmp_fn_result_call_line.ends_with(tmp_fn_result_var) { + tmp_fn_result_call_line = tmp_fn_result_call_line[..tmp_fn_result_call_line.len - tmp_fn_result_var.len] + } + g.empty_line = true + tmp_fn_result_call_value = g.new_tmp_var() + g.write('${g.styp(node.return_type)} ${tmp_fn_result_call_value} = ${tmp_fn_result_var}') + } mut effective_return_type := node.return_type if (gen_or || gen_keep_alive) && node.return_type != 0 { mut ret_typ := effective_return_type @@ -1969,6 +2003,17 @@ fn (mut g Gen) call_expr(node ast.CallExpr) { } else { g.fn_call(node) } + if needs_tmp_fn_result_cleanup && !gen_or && !gen_keep_alive && !g.inside_curry_call { + if node.return_type == ast.void_type { + g.writeln(';') + g.write('builtin__closure__closure_try_destroy((voidptr)${tmp_fn_result_var})') + g.set_current_pos_as_last_stmt_pos() + } else if tmp_fn_result_call_value != '' { + g.writeln(';') + g.writeln('builtin__closure__closure_try_destroy((voidptr)${tmp_fn_result_var});') + g.write2(tmp_fn_result_call_line, tmp_fn_result_call_value) + } + } if gen_or && node.return_type != 0 { g.or_block(tmp_opt, node.or_block, effective_return_type) mut unwrapped_typ := effective_return_type.clear_option_and_result() diff --git a/vlib/v/markused/markused.v b/vlib/v/markused/markused.v index 7ad5e6dc1..7c8b1e974 100644 --- a/vlib/v/markused/markused.v +++ b/vlib/v/markused/markused.v @@ -121,6 +121,7 @@ pub fn mark_used(mut table ast.Table, mut pref_ pref.Preferences, ast_files []&a core_fns << 'builtin.closure.closure_init' core_fns << 'builtin.closure.closure_create' core_fns << 'builtin.closure.closure_data' + core_fns << 'builtin.closure.closure_try_destroy' } if table.used_features.arr_map { include_panic_deps = true diff --git a/vlib/v/tests/fns/closure_direct_call_reuses_slot_test.c.v b/vlib/v/tests/fns/closure_direct_call_reuses_slot_test.c.v new file mode 100644 index 000000000..a35182c66 --- /dev/null +++ b/vlib/v/tests/fns/closure_direct_call_reuses_slot_test.c.v @@ -0,0 +1,47 @@ +@[has_globals] +module main + +$if windows { + #include + + struct C.builtin__closure__ClosureMutex { + closure_mtx C.SRWLOCK + } +} $else { + #include + + @[typedef] + pub struct C.pthread_mutex_t {} + + struct C.builtin__closure__ClosureMutex { + closure_mtx C.pthread_mutex_t + } +} + +struct C.builtin__closure__Closure { + ClosureMutex C.builtin__closure__ClosureMutex + closure_ptr voidptr + closure_get_data voidptr + closure_cap int + v_page_size int + free_closure_ptr voidptr +} + +__global C.g_closure C.builtin__closure__Closure + +fn foo(i int) fn () int { + return fn [i] () int { + return i + } +} + +fn test_direct_call_of_returned_closure_reuses_closure_slot() { + assert foo(-1)() == -1 + start_closure_cap := C.g_closure.closure_cap + mut sum := 0 + for i in 0 .. 128 { + sum += foo(i)() + } + assert sum == 8128 + assert C.g_closure.closure_cap == start_closure_cap +} -- 2.39.5