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

Fix memory leak in emp::DataFile #467

Open
FergusonAJ opened this issue May 18, 2022 · 0 comments
Open

Fix memory leak in emp::DataFile #467

FergusonAJ opened this issue May 18, 2022 · 0 comments

Comments

@FergusonAJ
Copy link
Contributor

Bug description
One of the constructors in emp::DataFile doesn't take an std::ostream, instead it takes a filename string.
It then uses that string to create a new std::ostream pointer. However, this pointer (os) is never deleted. Since it's not an Empirical pointer, this is not detected in debug mode.

Relevant code (line 52 at time of writing):

    DataFile(const std::string & in_filename,
             const std::string & b="", const std::string & s=",", const std::string & e="\n")
      : filename(in_filename), os(
      #ifdef __EMSCRIPTEN__
      new emp::NullStream()
      #else
      new std::ofstream(in_filename)
      #endif
      ), funs(), pre_funs(), keys(), descs()
      , timing_fun([](size_t){return true;})
      , line_begin(b), line_spacer(s), line_end(e) { ; }

To Reproduce
The DataFile example (examples/data/DataFile.cpp) has this issue without any modifications.
Here's the valgrind command I used to show the issue:

valgrind --leak-check=full --show-reachable=yes --leak-resolution=high --num-callers=100 --trace-children=yes ./DataFile 

Expected behavior
Removing the memory leak is the main priority.
Switching the os pointer to be an emp::Ptr would improve consistency with rest of codebase.
Destructor and copy/move constructors will likely need to be updated to accommodate issue (rule of five).

Toolchain (please complete the following information):

  • OS: Pop!_OS 21.04
  • Compiler: gcc version 10.3.0 (Ubuntu 10.3.0-1ubuntu1)
  • Empirical Version master at 1fe7f90
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant