From 6fb42185648ac22260bb7b40767335960572d169 Mon Sep 17 00:00:00 2001 From: Alexander Medvednikov Date: Wed, 25 Mar 2026 16:42:16 +0300 Subject: [PATCH] veb: fix memory leak (fixes #25518) --- vlib/veb/tests/memory_leak_test.v | 52 +++++++++++++++++++++++- vlib/veb/tests/memory_leak_test_server.v | 11 ++++- vlib/veb/veb.v | 6 +++ vlib/veb/veb_picoev.v | 29 +++++++++++-- 4 files changed, 92 insertions(+), 6 deletions(-) diff --git a/vlib/veb/tests/memory_leak_test.v b/vlib/veb/tests/memory_leak_test.v index 92cea4b73..a2e8f003f 100644 --- a/vlib/veb/tests/memory_leak_test.v +++ b/vlib/veb/tests/memory_leak_test.v @@ -33,7 +33,17 @@ fn test_server_compiles() { fn test_server_runs_in_background() { spawn os.system('${os.quoted_path(serverexe)} ${sport} ${exit_after_time}') - time.sleep(500 * time.millisecond) + for _ in 0 .. 50 { + resp := http.get('http://${localserver}/heap') or { + time.sleep(100 * time.millisecond) + continue + } + if resp.status_code == 200 { + return + } + time.sleep(100 * time.millisecond) + } + assert false, 'Timed out waiting for background server to start' } fn test_large_responses_work_correctly() { @@ -68,3 +78,43 @@ fn test_gc_collect_endpoint_works() { } assert resp.status_code == 200 } + +fn test_multipart_upload_does_not_expand_heap_excessively() { + http.get('http://${localserver}/gc') or { + assert false, 'Failed to trigger GC before measuring heap: ${err}' + return + } + before_resp := http.get('http://${localserver}/heap') or { + assert false, 'Failed to get baseline heap usage: ${err}' + return + } + before_heap := before_resp.body.i64() + payload_size := 1024 * 1024 + mut files := []http.FileData{} + files << http.FileData{ + filename: 'payload.bin' + content_type: 'application/octet-stream' + data: 'X'.repeat(payload_size) + } + resp := http.post_multipart_form('http://${localserver}/upload', http.PostMultipartFormConfig{ + files: { + 'file': files + } + }) or { + assert false, 'Failed to post multipart upload: ${err}' + return + } + assert resp.status_code == 200 + assert resp.body == payload_size.str() + http.get('http://${localserver}/gc') or { + assert false, 'Failed to trigger GC after upload: ${err}' + return + } + after_resp := http.get('http://${localserver}/heap') or { + assert false, 'Failed to get post-upload heap usage: ${err}' + return + } + after_heap := after_resp.body.i64() + heap_growth := if after_heap > before_heap { after_heap - before_heap } else { i64(0) } + assert heap_growth < 8 * 1024 * 1024, 'Multipart upload grew heap by ${heap_growth} bytes' +} diff --git a/vlib/veb/tests/memory_leak_test_server.v b/vlib/veb/tests/memory_leak_test_server.v index 9e4749b4d..dc3c6848e 100644 --- a/vlib/veb/tests/memory_leak_test_server.v +++ b/vlib/veb/tests/memory_leak_test_server.v @@ -27,7 +27,7 @@ fn main() { mut app := &App{} veb.run_at[App, Context](mut app, - host: 'localhost' + host: '127.0.0.1' port: http_port family: .ip timeout_in_seconds: 10 @@ -55,3 +55,12 @@ pub fn (mut app App) large(mut ctx Context) veb.Result { data := 'X'.repeat(100 * 1024) return ctx.text(data) } + +@['/upload'; post] +pub fn (mut app App) upload(mut ctx Context) veb.Result { + if 'file' !in ctx.files || ctx.files['file'].len == 0 { + ctx.res.set_status(.bad_request) + return ctx.text('no file') + } + return ctx.text(ctx.files['file'][0].data.len.str()) +} diff --git a/vlib/veb/veb.v b/vlib/veb/veb.v index 66597b983..d54ea43a7 100644 --- a/vlib/veb/veb.v +++ b/vlib/veb/veb.v @@ -410,6 +410,8 @@ $if !new_veb ? { mut: // request body buffer buf &u8 = unsafe { nil } + // request bodies are assembled in byte buffers to avoid repeated string reallocations + body_buffers [][]u8 // idx keeps track of how much of the request body has been read // for each incomplete request, see `handle_conn` idx []int @@ -424,6 +426,10 @@ $if !new_veb ? { pub fn (mut params RequestParams) request_done(fd int) { mut request := ¶ms.incomplete_requests[fd] request.reset() + if params.body_buffers[fd].cap > 0 { + unsafe { params.body_buffers[fd].free() } + params.body_buffers[fd] = []u8{} + } params.idx[fd] = 0 $if trace_handle_read ? { eprintln('>>>>> fd: ${fd} | request_done.') diff --git a/vlib/veb/veb_picoev.v b/vlib/veb/veb_picoev.v index c83a87881..80695d3bf 100644 --- a/vlib/veb/veb_picoev.v +++ b/vlib/veb/veb_picoev.v @@ -41,6 +41,7 @@ $if !new_veb ? { // reserve space for read and write buffers pico_context.buf = unsafe { malloc_noscan(picoev.max_fds * max_read + 1) } defer { unsafe { free(pico_context.buf) } } + pico_context.body_buffers = [][]u8{len: picoev.max_fds} pico_context.incomplete_requests = []http.Request{len: picoev.max_fds} pico_context.file_responses = []FileResponse{len: picoev.max_fds} pico_context.string_responses = []StringResponse{len: picoev.max_fds} @@ -177,6 +178,26 @@ $if !new_veb ? { return error('invalid chunked body') } + @[inline] + fn append_request_body(mut params RequestParams, fd int, chunk []u8, expected_len int) { + if chunk.len == 0 { + return + } + if params.body_buffers[fd].cap == 0 { + initial_cap := if expected_len > 0 { expected_len } else { chunk.len } + params.body_buffers[fd] = []u8{cap: initial_cap} + } + params.body_buffers[fd] << chunk + } + + @[inline; manualfree] + fn finalize_request_body(mut params RequestParams, fd int) string { + body := params.body_buffers[fd].bytestr() + unsafe { params.body_buffers[fd].free() } + params.body_buffers[fd] = []u8{} + return body + } + // handle_write_file reads data from a file and sends that data over the socket. @[direct_array_access; manualfree] fn handle_write_file(mut pv picoev.Picoev, mut params RequestParams, fd int) { @@ -427,17 +448,17 @@ $if !new_veb ? { return } else if n < bytes_to_read || params.idx[fd] + n < content_length_i { // request is incomplete wait until the socket becomes ready to read again - // TODO: change this to a memcpy function? - req.data += buf[0..n].bytestr() + append_request_body(mut params, fd, buf[0..n], content_length_i) params.incomplete_requests[fd] = req params.idx[fd] += n $if trace_handle_read ? { - eprintln('>>>>> request is NOT complete, fd: ${fd} | n: ${n} | req.data.len: ${req.data.len} | params.idx[fd]: ${params.idx[fd]}') + eprintln('>>>>> request is NOT complete, fd: ${fd} | n: ${n} | body_buffer.len: ${params.body_buffers[fd].len} | params.idx[fd]: ${params.idx[fd]}') } return } else { // request is complete: n = bytes_to_read - req.data += buf[0..n].bytestr() + append_request_body(mut params, fd, buf[0..n], content_length_i) + req.data = finalize_request_body(mut params, fd) params.idx[fd] += n $if trace_handle_read ? { eprintln('>>>>> request is NOW COMPLETE, fd: ${fd} | n: ${n} | req.data.len: ${req.data.len}') -- 2.39.5