Opened 10 years ago
Closed 10 years ago
#8079 closed Bugs (fixed)
Chrono memory leak
Reported by: | Owned by: | viboes | |
---|---|---|---|
Milestone: | Boost 1.54.0 | Component: | chrono |
Version: | Boost 1.53.0 | Severity: | Problem |
Keywords: | Cc: |
Description
The constructor for duration_units_default_initializer_t (in chrono/io/duration_units.hpp) allocates two string_type arrays from the heap but never deletes them. The Visual C++ MFC debugger reports this as a memory leak (and I expect most other leak detectors will as well).
These variables are class static and only one instance of this class (per CharT) is allocated anyway, so this is not a "serious" memory leak, but any memory leak reported by a leak detector is worth fixing to avoid masking other more serious memory leaks.
I suggest adding a destructor similar to this:
~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; } }
Further suggestions for this code:
- Make the class static member variables n_d_valid_units_ and valid_units_ into non-static member variables. Only one object of this type is created, so making them them regular member variables makes things clearer and simplifies the code.
- Do away with the initialized_ member variable entirely. If you make the other member variables non-static, there is no need for this variable.
- If I am wrong and it is possible to create more than one instance of duration_units_default_initializer_t<CharT>, then you will need to make the destructor more intelligent and use smart pointers or a reference count to know when to destroy the static member variables.
Change History (4)
comment:1 by , 10 years ago
comment:2 by , 10 years ago
I have not tested in a multi-DLL environment, I have been working with statically linked boost. Off-hand I don't see a problem -- wouldn't each DLL have it's own copy of the static variables?
comment:3 by , 10 years ago
Milestone: | To Be Determined → Boost 1.54.0 |
---|---|
Status: | new → assigned |
Your are right. Committed in trunk [83510].
You are not wrong, there is only one instance.
Changing all the static members to non-static implies to store a pointer to such a single object. Maybe this could be more elegant, but this adds an indirection. I would not be against a patch, but currently I have no time for such a refactoring.
Have you checked your suggestion of deleting the allocated memory on the destructor in a multi DLL environment?