Skip to content
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.

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

mitigate memory leak in http client due to unreliable lazy parameter spec #36

Closed
wants to merge 1 commit into from

Conversation

Boris-Barboris
Copy link

When http client is used heavily with high probability of failed requests (400 response from the remote server is sufficient in my case), VIRT/RSS of the vibe-d server process raises infinitely under dmd 2.083, even when loglevel is not debug. Two things are obvious from Heaptrack traces:

  1. logDebug's second parameter is not lazified.
  2. e.toString().sanitize() leaks memory.

@s-ludwig
Copy link
Member

I've taken a look at the logging code and it seems like vibe-d/vibe-core#e769a9b1ca8076e4aeceb573c86140a950f9cf8f has introduced a regression w.r.t. lazy evaluation of the format arguments. Instead of always making a copy of the arguments, it should first check if any logger actually has the required minLevel. I'll open a PR there to fix this at its root!

sanitize() shouldn't leak memory per se (and should only allocate if there is actually anything to sanitize), but I guess that this is an instance of the GC not being able to reuse pool memory (maybe due to memory fragmentation, or maybe due to a bug). I've observed that lots of times in the past in situation where memory was being allocated rapidly from multiple threads.

In any case, this is a good catch!

s-ludwig added a commit to vibe-d/vibe-core that referenced this pull request Apr 16, 2024
s-ludwig added a commit to vibe-d/vibe-core that referenced this pull request Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants