Re: [Boost-users] [Chrono] Memory leak in Chrono?

-------- Original Message -------- Subject: Boost-users Digest, Vol 3358, Issue 4 From: boost-users-request@lists.boost.org To: boost-users@lists.boost.org Date: 2/13/2013 1:33 PM
Subject: Re: [Boost-users] [Chrono] Memory leak in Chrono? From: "Vicente J. Botet Escriba"
Date: 2/13/2013 1:11 PM Le 13/02/13 14:30, Don a écrit :
Subject: Re: [Boost-users] [Chrono] Memory leak in Chrono? From: "Vicente J. Botet Escriba"
Date: 2/12/2013 1:17 PM Le 12/02/13 18:13, gast128 a écrit :
we use Boost.Chrono too. MFC applications incorrectly report memory leaks, which can easily be solved. Hi,
if you have some patches to propose to avoids this incorrect reports I would try to include them.
Vincente,
I am using MFC, but I don't believe this is a MFC false-leak report.
I added a destructor for duration_units_default_initializer_t, like this (near the end of chrono/io/duration_units.hpp):
~duration_units_default_initializer_t() { if (duration_units_default_holder<CharT>::initialized_) { delete[] duration_units_default_holder<CharT>::n_d_valid_units_; duration_units_default_holder<CharT>::n_d_valid_units_ = 0; delete[] duration_units_default_holder<CharT>::valid_units_; duration_units_default_holder<CharT>::valid_units_ = 0; duration_units_default_holder<CharT>::initialized_ = false; } }
This solves the memory leak, and seems to work fine for my simple test program. I could add this is this helps on the MFC report.
Thanks, that would be useful. It would probably help with other memory leak detectors, as well. Since you are allocating heap that you are never freeing, most leak detectors are going to report this.
But n_d_valid_units_, valid_units_, and initialized_ are static variables, so I assume you want to only create these variables once.
Right.
When run with my test program and step through it in the debugger, only one instantiation of duration_units_default_initializer_t occurs. (That is, once for each char type, single-byte and wide.)
This is the goal of the type (implement a singleton). This technique is used in some Boost libraries already
This occurs when the duration_units_default_initializer is instantiated (at the very end of duration_units.hpp). Since this is a global variable, this occurs during application startup and the destructor runs during application shuttdown.
Right, this was the reason the destructor didn't do anything.
But presumably this can occur in some other situations, otherwise why would you have all the logic of making valid_units_ etc. class-static variables.>
On which other situation do you think this could occur?
I have no idea, that's what confused me. If duration_units_default_initializer_t is only instantiated as a global (static) variable), then I don't see any reason for making n_d_valid_units_, valid_units_, and initialized_ static variables -- they could be regular member variables. Making them static doesn't hurt, but it's confusing (to me at least) because it looks like you want to share those variables among multiple instantiations, and there is only ever one instantiation. If one can somehow instantiate multiple duration_units_default_initializer_t objects, then making the member variables static does make this behave as a singleton. In that case the simple destructor I wrote will not work, since it will delete the member variables when the first object destructs, not the last. In that case I would use a reference count or smart pointers to only delete when the final object destructs. Thanks for your help, I appreciate your spending the time to answer my questions. Don

-------- Original Message -------- Subject: Boost-users Digest, Vol 3358, Issue 4 From: boost-users-request@lists.boost.org To: boost-users@lists.boost.org Date: 2/13/2013 1:33 PM
Subject: Re: [Boost-users] [Chrono] Memory leak in Chrono? From: "Vicente J. Botet Escriba"
Date: 2/13/2013 1:11 PM Le 13/02/13 14:30, Don a écrit :
Subject: Re: [Boost-users] [Chrono] Memory leak in Chrono? From: "Vicente J. Botet Escriba"
Date: 2/12/2013 1:17 PM Le 12/02/13 18:13, gast128 a écrit :
we use Boost.Chrono too. MFC applications incorrectly report memory leaks, which can easily be solved. Hi,
if you have some patches to propose to avoids this incorrect reports I would try to include them.
Vincente,
I am using MFC, but I don't believe this is a MFC false-leak report.
I added a destructor for duration_units_default_initializer_t, like this (near the end of chrono/io/duration_units.hpp):
~duration_units_default_initializer_t() { if (duration_units_default_holder<CharT>::initialized_) { delete[] duration_units_default_holder<CharT>::n_d_valid_units_; duration_units_default_holder<CharT>::n_d_valid_units_ = 0; delete[] duration_units_default_holder<CharT>::valid_units_; duration_units_default_holder<CharT>::valid_units_ = 0; duration_units_default_holder<CharT>::initialized_ = false; } }
This solves the memory leak, and seems to work fine for my simple test program. I could add this is this helps on the MFC report.
Thanks, that would be useful. It would probably help with other memory leak detectors, as well. Since you are allocating heap that you are never freeing, most leak detectors are going to report this. My intention was not to add the code as such, but add it only to avoid
Le 14/02/13 01:52, Don a écrit : the MFC detector, using conditional compilation.
But n_d_valid_units_, valid_units_, and initialized_ are static variables, so I assume you want to only create these variables once.
Right.
When run with my test program and step through it in the debugger, only one instantiation of duration_units_default_initializer_t occurs. (That is, once for each char type, single-byte and wide.)
This is the goal of the type (implement a singleton). This technique is used in some Boost libraries already
This occurs when the duration_units_default_initializer is instantiated (at the very end of duration_units.hpp). Since this is a global variable, this occurs during application startup and the destructor runs during application shuttdown.
Right, this was the reason the destructor didn't do anything.
But presumably this can occur in some other situations, otherwise why would you have all the logic of making valid_units_ etc. class-static variables.>
On which other situation do you think this could occur?
I have no idea, that's what confused me. If duration_units_default_initializer_t is only instantiated as a global (static) variable), then I don't see any reason for making n_d_valid_units_, valid_units_, and initialized_ static variables -- they could be regular member variables. Making them static doesn't hurt, but it's confusing (to me at least) because it looks like you want to share those variables among multiple instantiations, and there is only ever one instantiation. ,
If one can somehow instantiate multiple duration_units_default_initializer_t objects, then making the member variables static does make this behave as a singleton. In that case the simple destructor I wrote will not work, since it will delete the member variables when the first object destructs, not the last. In that case I would use a reference count or smart pointers to only delete when the final object destructs.
Please, could you create a Trac ticket so this issue not lost? Best, Vicente
participants (2)
-
Don
-
Vicente J. Botet Escriba