ls1mardyn / ls1-mardyn

ls1-MarDyn is a massively parallel Molecular Dynamics (MD) code for large systems. Its main target is the simulation of thermodynamics and nanofluidics. ls1-MarDyn is designed with a focus on performance and easy extensibility.
http://www.ls1-mardyn.de
Other
28 stars 15 forks source link

Replace raw with smart pointers #337

Closed HomesGH closed 3 weeks ago

HomesGH commented 2 months ago

Description

The code scanning revealed some "Resource not released in destructor" warnings. This PR intends to fix them.

BTW: Is bhfmm still in use/under development? I couldn't find any documentation regarding what it does. Maybe you know more, @cniethammer

FG-TUM commented 2 months ago

I can give you some context: The bhfmm is the fast multipole method for long range interactions that was developed primarily by Nikola (I did the OpenMP parallelization of it as my master's thesis). As far as I know this code has not been touched since he left so it is unknown if it is still working (correctly). It is also not tested by the CI. So regarding your questions:

HomesGH commented 1 month ago

It is not that straightforward to replace the raw pointer to the Logger (global_log) with a smart pointer. Maybe somebody has any advice? My current implementation in this PR leads to a segmentation fault at the very end of the simulation (free(): invalid pointer). I guess that the pointer is tried to be deleted but was already cleaned up before. Maybe it has something to do with the namespace Log?

HomesGH commented 1 month ago

It is not that straightforward to replace the raw pointer to the Logger (global_log) with a smart pointer. Maybe somebody has any advice? My current implementation in this PR leads to a segmentation fault at the very end of the simulation (free(): invalid pointer). I guess that the pointer is tried to be deleted but was already cleaned up before. Maybe it has something to do with the namespace Log?

The issue was with _log_stream. This could either be std::cout which is globally managed and should not be deleted as it is managed by the runtime or a dynamically allocated ofstream. The issue was solved by introducing a custom deleter during initialization.