-
Notifications
You must be signed in to change notification settings - Fork 173
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
Fix/memory leaks #846
Fix/memory leaks #846
Conversation
- add manual resource clean up to Runtime::close(); - make certain task termination a bit cleaner;
…time, if there any active blocking tasks left
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good for me. But I don't have enough experience with zenoh deep internals and with tokyo, so it would be good to have this PR reviewed by someone from protocol team. @OlivierHecart ?
The only concern: I'd prefer graceful failure instead of unwrap()
panics. In most cases it's possible to just return empty result as far as I see
…re clarity; - Some tasks now terminate in more graceful way
@DenisBiryukov91 , I'd like to leave some comments since you're addressing some issues that I wanted to solve a few weeks ago on Tokio porting. Here're some suggestions
|
- terminate more tasks - make TaskController::terminate_all[_async] accept timeout duration
…-pool Improve the drop of ZRUNTIME_POOL
Addresses a series of memory leaks detected by valgrind (Closes #695 ):
due to cyclic references in Runtime and its dependents
due to cyclic references in Session and its dependents
due to cyclic references in Resource and its children
due to cyclic reference in Mux->Face->State->Mux
due to non-terminated tasks after Session.close()
Valgrind still reports leaks, related to static variable ZRUNTIME_POOL. This is because rust never calls drop on static variables by design. This leaks can be removed by calling zruntime_pool_free() or using ZRuntimePoolGuard.
Also adds a valgrind memory leaks test into ci.