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

Memory Leak in Comm::wait_any_for() when killing the waiting actor during the wait #308

Closed
henricasanova opened this issue Nov 3, 2018 · 4 comments
Labels

Comments

@henricasanova
Copy link
Contributor

I am not sure this can be fixed in any way, but I have a simulation in which actors do wait_any_for() a lot, and also kill each other brutally a lot. As a result, in wait_any_for() the dynar allocated as

xbt_dynar_t comms = xbt_dynar_new(sizeof(simgrid::kernel::activity::ActivityImpl*), [](void* ptr) {
intrusive_ptr_release(*(simgrid::kernel::activity::ActivityImpl**)ptr);
});

leaks if the actor is killed why it is waiting (the dynar is freed only at the end of the wait_any_for() code).

Again, this seems difficult to fix and after all killing actors brutally may not be good design, but I thought I'd mention it just in case Mt can do some magic ;)

@mquinson mquinson changed the title Memory Leak in Comm::wait_any_for() Memory Leak in Comm::wait_any_for() when killing the waiting actor during the wait Oct 16, 2019
@mquinson
Copy link
Member

I would not be surprised if this was fixed in between. The line now reads

  std::unique_ptr<kernel::activity::CommImpl* []> rcomms(new kernel::activity::CommImpl*[comms->size()]);

so I think that RAII is now allowing to free the resource when the exception is used to (1) kill the actor (2) clean all data allocated on the stack.

@henricasanova if you have a easy way to reproduce this bug (even if not a MWE), could you please check whether I am right? Thanks.

@mquinson
Copy link
Member

mquinson commented Mar 1, 2020

Hello @henricasanova, if you get bored in the future, could you please check whether the issue got fixed in between, as I think? If not, could we maybe close the bug since we fail to reproduce and think it's fixed anyway?

Thanks,

@henricasanova
Copy link
Contributor Author

Sorry to have missed this. It's been 3 years now that I've posted this. I think I was able to reproduce it using an old MWE about this, and valgrind confirms that there are no leaks. So I believe it's safe to close. Doing so right now.

@mquinson
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants