AVSLab / basilisk

Astrodynamics simulation framework
https://hanspeterschaub.info/basilisk
ISC License
135 stars 60 forks source link

Spacecraft module appears to leak memory #441

Open ascended121 opened 1 year ago

ascended121 commented 1 year ago

Describe the bug

Spacecraft::Spacecraft() calls new on svIntegratorRK4, but never frees it.

To reproduce Steps to reproduce the behavior:

  1. Run any code invoking the Spacecraft under valgrind
  2. See messages about memory leaks

Expected behavior The code should not leak memory.

Screenshots N/A

Desktop:

OS: Ubuntu 20.04 Version: 20.04 Python version: 3.8

Additional context It appears that the module may need to call delete this->integrator; in its destructor.

ascended121 commented 1 year ago

The SpacecraftSystem appears to do the same thing.

schaubh commented 9 months ago

Thanks, I agree that the destructor should free this. I'm creating a branch to address this.

Mark2000 commented 9 months ago

@ascended121 I'm digging into some other suspected memory leaks; do you have any more details on how you are setting up valgrind?

ascended121 commented 9 months ago

@schaubh When using Valgrind with python, you need to use a suppression file provided by the python devs, to remove some false positives from the results: http://svn.python.org/projects/python/trunk/Misc/valgrind-python.supp Source

Also, you need to tell python to use malloc (instead of its own internal memory management), in order for valgrind to track the allocations. Since python 3.6, this can be done via the PYTHONMALLOC env variable. Source

So, combining those (assuming you've downloaded the suppressions locally), you should be able to run:

$ PYTHONMALLOC=malloc valgrind --suppressions=valgrind-python.supp --leak-check=full python3 src/utilities/MonteCarlo/_UnitTests/test_scenarioBasicOrbitMC.py

This example runs the MonteCarlo tests but that can be replaced with whichever unit test or scenario you're actually interested in.

When I try to run the command above to reproduce the original output showing the memory leak, I'm running into this bug (ie, valgrind can't parse the DWARF5 output of clang14). I'm not inclined to downgrade my version of clang (I recently updated to clang14), and valgrind-3.20 (which fixes that bug) is not available as a package for Ubuntu 22.04 and I'm disinclined to compile it from source, so I'll leave this here and hope you've got compatible versions of clang/valgrind and can make this work!