Memory leak in Signal

[Aneoshun]

Bug Report

Please answer the following questions for yourself before reporting a bug.

• I checked the documentation and the forum but found no answer.
• I checked to make sure that this issue has not already been filed.

Environment

Select the following information.

• DART version: master
• OS name and version name(or number): Ubuntu 18.04
• Compiler name and version number: GCC 7.4.0

Expected Behavior

Please describe the behavior you are expecting.
Creating a skeleton, then repeatedly cloning and deleting the clones should have a constant memory footprint.

Current Behavior

What is the current behavior?

Currently, the memory footprint grows linearly with the number of clones created and deleted.
Here is what Valgrind with Massif gives:

As you can see the very vast majority of the memory (99.5%) is used by dart::common::Signal<void (dart::dynamics::Shape*, unsigned long), dart::common::signal::detail::DefaultCombiner>

Steps to Reproduce

Please provide detailed steps for reproducing the issue.
You can use the following example to reproduce this behavior:

#include <iostream>
#include <unistd.h>

#include <dart/utils/SkelParser.hpp>
#include <dart/utils/sdf/SdfParser.hpp>
#include <dart/utils/urdf/urdf.hpp>


namespace global{
  dart::dynamics::SkeletonPtr skel;
}

void load_and_init_robot(std::string path)
{
  std::string model_file = path+"hexapod_v2.urdf"; //to be replaced with any valid urdf
  model_file.erase(model_file.begin(), std::find_if(model_file.begin(), model_file.end(), [](int ch) {
	return !std::isspace(ch);
      }));
  
  if (model_file[0] != '/') {
    constexpr size_t max_size = 512;
    char buff[max_size];
    auto val = getcwd(buff, max_size);
    model_file = std::string(buff) + "/" + model_file;
  }
  
  
  // Load from URDF string
  dart::utils::DartLoader loader;
  global::skel = loader.parseSkeleton(model_file);

}

int main(int argc, char **argv) 
{
  load_and_init_robot("exp/memory_hunt/src/hexapod_omnidirectional/");

  size_t nb_clones = 5000;
  
  for(size_t i = 0; i<nb_clones; i++)
    {
      std::cout<<"gen "<<i<<"/"<<nb_clones<<std::endl;
      global::skel->getMutex().lock();
      auto tmp_skel = global::skel->cloneSkeleton();
      global::skel->getMutex().unlock();
      tmp_skel.reset();
   }
  
  global::skel.reset();
  return 0;
}

The program can be executed with valgrind --tool=massif to produced the reports shown above.

Found Steps to Fix it

After analysing the code, I found out that the Signal shape->onVersionChanged used in the ShapeFrames keeps adding the "ConnectionBodies" in its set but never actually clear them when the connection is disconnected: mConnectionBodies.insert(newConnectionBody);
Then the connection are disconnected, they are flagged as "disconnected", but never removed from "mConnectionBodies".

Signal as a function cleanupConnections which would solve the problem, but it seems that this function is never used.
I don't know what is the cleanest way to manage these disconnected ConnectionBodies, but I found out that adding cleanupConnections();
after every insertion mConnectionBodies.insert(newConnectionBody); solves the issue, as demonstrated in the following graph, with a constant memory footprint lower than 1MB

I hope this report will help you fixing this.
Thanks for your work!

Cheers,

[jslee02]

Thank you for the detailed report! This is very helpful.

By design, the current Signal doesn't clean up the disconnected connections until the signal calls the registered slots or explicitly calls cleanupConnections(). Technically it's not a memory leak, but it could be problematic in the case like the case of this report.

#1462 should resolve the issue. Could you please confirm that PR fixes the issue?

[Aneoshun]

Thanks a lot @jslee02. I have tested on my side, and I can confirm that #1462 resolves the issue.

[jslee02]

Awesome! Closing since the issue is resolved.