Fix memory leak#59
Conversation
| let semaphore = Arc::new(( | ||
| std::sync::Mutex::new(0usize), | ||
| std::sync::Condvar::new(), | ||
| )); |
There was a problem hiding this comment.
[ultra-nit] We might want to put this in a new type and add something like .inc and .dec methods to remove some verbosity in the loop. But honestly, this way is fine as well.
Co-authored-by: Mauro Ezequiel Moltrasio <moltrasiom@hotmail.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@JoukoVirtanen @Molter73 I was about to merge #56, when I realized this PR conflicts with it. As far as I can tell the memory leak fix in this PR is to keep only one last instance of the random path, which is very specific and not extendable. I would suggest adopt the reference counting approach from the #56, in particularly because it also enables memory sanitizer. Can any of you rebase this PR on top of the latest changes and resolve the conflict? |
| let (lock, cvar) = &*semaphore; | ||
| let mut count = cvar | ||
| .wait_while(lock.lock().unwrap(), |c| { | ||
| *c >= MAX_CONCURRENT |
There was a problem hiding this comment.
Do I see it correctly that this limit maximum number of parallel workers?
Currently threads created in
run_payloadare not closed resulting in a memory leak. This PR fixes that.This PR also fixes a memory leak resulting from
CString::new(format!("{base}/{uniq}")).unwrap().into_raw().