Skip to content

veb: fix two memory leaks #26236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
fleximus wants to merge 2 commits into
base: master
Choose a base branch
from
Open

veb: fix two memory leaks #26236

fleximus wants to merge 2 commits into from
+128 −8

Conversation

@fleximus
Copy link
Contributor

@fleximus fleximus commented Jan 1, 2026

  1. The veb web framework had a memory leak where response bodies were being cloned multiple times, causing unbounded memory growth under load
  2. Additionally, when veb_livereload was enabled, the first clone was immediately overwritten by response.replace(...), leaking the first allocation

New tests:

  • memory_leak_test.v - Integration test that verifies large responses work correctly
  • memory_leak_test_server.v - Test server with endpoints for heap monitoring and GC collection

Fixes #25423
@fleximus
veb: fix memory leaks
4f17481 
  1. The veb web framework had a memory leak where response bodies were being cloned multiple times,
     causing unbounded memory growth under load.
  2. Additionally, when `veb_livereload` was enabled, the first clone was immediately overwritten
     by `response.replace(...)`, leaking the first allocation.

New tests:
  - `memory_leak_test.v` - Integration test that verifies large responses work correctly
  - `memory_leak_test_server.v` - Test server with endpoints for heap monitoring and GC collection
@Avey777
Copy link
Contributor

Avey777 commented Jan 1, 2026

picoev will be replaced by fasthttp.
@fleximus
Copy link
Contributor Author

fleximus commented Jan 1, 2026

picoev will be replaced by fasthttp.

Oh I just saw the 0.5.0 realease 😃
What will mean it to this fix? Patching the legacy?
@JalonSolov
Copy link
Contributor

JalonSolov commented Jan 1, 2026 •
edited
Loading

Looks like these changes are specific to veb, not picoev or fasthttp (which would be lower-level protocol handlers). If this PR is approved, there's no reason not to merge it as-is.

Yes, you did touch picoev, but just to remove commented out lines... no harm, no foul (and arguably should've been done long ago).
spytheman
spytheman reviewed Jan 1, 2026
View reviewed changes
@spytheman
Copy link
Member

spytheman commented Jan 1, 2026

picoev will be replaced by fasthttp.

Oh I just saw the 0.5.0 realease 😃 What will mean it to this fix? Patching the legacy?

No, this PR is not affected in any way, since we do not do backports to older versions - once they are done, they are immutable.

The fix from this PR, will be merged in the master branch, and will be available right away after that, to people that do v up regularly. It will be available next Monday, for people that download weekly releases (or from the website, which also uses them), and in 2-3 months, when @medvednikov does another versioned release.
@fleximus
Copy link
Contributor Author

fleximus commented Jan 1, 2026

@spytheman what do you think about the failing Docker CI?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants

@fleximus @Avey777 @JalonSolov @spytheman