Skip to content

Commit

Permalink
BUG: Fix memory leaks in SubjectHierarchy Register and Segment plugins
Browse files Browse the repository at this point in the history
Since QMenu constructor does not take a QObject as a parent, this commit
introduces a QSharedPointer to manage the menu instance.

This commit fixes the error(s) reported below by valgrind memcheck tool.

Valgrind was used on Ubuntu 14.04 against a Debug build of Slicer. It was
exected doing the following:
 (1) Start a terminal with the appropriate environment: ./Slicer --gnome-terminal
 (2) Start Slicer using valgrind: valgrind --log-file=2015-07-16-Slicer-memcheck.txt --tool=memcheck --leak-check=yes ./bin/SlicerApp-real --disable-python --disable-cli-modules
 (3) Exit Slicer and inspect valgrind log file

Valgrind error:

==29339== 1,160 (40 direct, 1,120 indirect) bytes in 1 blocks are definitely lost in loss record 2,294 of 2,448
==29339==    at 0x4C2B0E0: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==29339==    by 0x10550A4D9: qSlicerSubjectHierarchySegmentPluginPrivate::init() (qSlicerSubjectHierarchySegmentPlugin.cxx:114)
==29339==    by 0x10550A3E7: qSlicerSubjectHierarchySegmentPlugin::qSlicerSubjectHierarchySegmentPlugin(QObject*) (qSlicerSubjectHierarchySegmentPlugin.cxx:103)
==29339==    by 0x1054F1217: qSlicerSubjectHierarchyPluginLogic::registerCorePlugins() (qSlicerSubjectHierarchyPluginLogic.cxx:139)
==29339==    by 0x1054F0DFD: qSlicerSubjectHierarchyPluginLogic::qSlicerSubjectHierarchyPluginLogic(QWidget*) (qSlicerSubjectHierarchyPluginLogic.cxx:117)
==29339==    by 0x1080E2F5C: qSlicerSubjectHierarchyModule::createLogic() (qSlicerSubjectHierarchyModule.cxx:137)
==29339==    by 0x8E715C1: qSlicerAbstractCoreModule::logic() (qSlicerAbstractCoreModule.cxx:280)
==29339==    by 0x8E70BAD: qSlicerAbstractCoreModule::initialize(vtkSlicerApplicationLogic*) (qSlicerAbstractCoreModule.cxx:99)
==29339==    by 0x8EA2A19: qSlicerModuleFactoryManager::loadModule(QString const&, QString const&) (qSlicerModuleFactoryManager.cxx:170)
==29339==    by 0x8EA2586: qSlicerModuleFactoryManager::loadModule(QString const&) (qSlicerModuleFactoryManager.cxx:110)
==29339==    by 0x40672A: (anonymous namespace)::SlicerAppMain(int, char**) (Main.cxx:193)
==29339==    by 0x406D30: main (Main.cxx:254)

==29339==
==29339== 1,176 (40 direct, 1,136 indirect) bytes in 1 blocks are definitely lost in loss record 2,296 of 2,448
==29339==    at 0x4C2B0E0: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==29339==    by 0x105507073: qSlicerSubjectHierarchyRegisterPluginPrivate::init() (qSlicerSubjectHierarchyRegisterPlugin.cxx:120)
==29339==    by 0x105506EC1: qSlicerSubjectHierarchyRegisterPlugin::qSlicerSubjectHierarchyRegisterPlugin(QObject*) (qSlicerSubjectHierarchyRegisterPlugin.cxx:104)
==29339==    by 0x1054F11ED: qSlicerSubjectHierarchyPluginLogic::registerCorePlugins() (qSlicerSubjectHierarchyPluginLogic.cxx:137)
==29339==    by 0x1054F0DFD: qSlicerSubjectHierarchyPluginLogic::qSlicerSubjectHierarchyPluginLogic(QWidget*) (qSlicerSubjectHierarchyPluginLogic.cxx:117)
==29339==    by 0x1080E2F5C: qSlicerSubjectHierarchyModule::createLogic() (qSlicerSubjectHierarchyModule.cxx:137)
==29339==    by 0x8E715C1: qSlicerAbstractCoreModule::logic() (qSlicerAbstractCoreModule.cxx:280)
==29339==    by 0x8E70BAD: qSlicerAbstractCoreModule::initialize(vtkSlicerApplicationLogic*) (qSlicerAbstractCoreModule.cxx:99)
==29339==    by 0x8EA2A19: qSlicerModuleFactoryManager::loadModule(QString const&, QString const&) (qSlicerModuleFactoryManager.cxx:170)
==29339==    by 0x8EA2586: qSlicerModuleFactoryManager::loadModule(QString const&) (qSlicerModuleFactoryManager.cxx:110)
==29339==    by 0x40672A: (anonymous namespace)::SlicerAppMain(int, char**) (Main.cxx:193)
==29339==    by 0x406D30: main (Main.cxx:254)

svn-url: http://viewvc.slicer.org/viewvc.cgi/Slicer4?view=revision&revision=24445
git-svn-id: http://svn.slicer.org/Slicer4/trunk@24445 3bd1e089-480b-0410-8dfb-8563597acbee
  • Loading branch information
jcfr committed Jul 17, 2015
1 parent bdf0004 commit ae453ef
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 10 deletions.
Expand Up @@ -74,6 +74,7 @@ class qSlicerSubjectHierarchyRegisterPluginPrivate: public QObject
public:
QAction* RegisterThisAction;
QAction* RegisterToAction;
QSharedPointer<QMenu> RegistrationMethodsSubMenu;
};

//-----------------------------------------------------------------------------
Expand Down Expand Up @@ -117,20 +118,20 @@ void qSlicerSubjectHierarchyRegisterPluginPrivate::init()
this->RegisterToAction = new QAction("Register * to this using...",q);

// Actions for the registration methods
QMenu* registrationMethodsSubMenu = new QMenu();
this->RegisterToAction->setMenu(registrationMethodsSubMenu);
this->RegistrationMethodsSubMenu = QSharedPointer<QMenu>(new QMenu());
this->RegisterToAction->setMenu(this->RegistrationMethodsSubMenu.data());

QAction* imageBasedRigidAction = new QAction("Rigid image-based registration",q);
QObject::connect(imageBasedRigidAction, SIGNAL(triggered()), q, SLOT(registerImageBasedRigid()));
registrationMethodsSubMenu->addAction(imageBasedRigidAction);
this->RegistrationMethodsSubMenu->addAction(imageBasedRigidAction);

QAction* imageBasedBSplineAction = new QAction("BSpline image-based registration",q);
QObject::connect(imageBasedBSplineAction, SIGNAL(triggered()), q, SLOT(registerImageBasedBSpline()));
registrationMethodsSubMenu->addAction(imageBasedBSplineAction);
this->RegistrationMethodsSubMenu->addAction(imageBasedBSplineAction);

QAction* interactiveLandmarkAction = new QAction("Interactive landmark registration",q);
QObject::connect(interactiveLandmarkAction, SIGNAL(triggered()), q, SLOT(registerInteractiveLandmark()));
registrationMethodsSubMenu->addAction(interactiveLandmarkAction);
this->RegistrationMethodsSubMenu->addAction(interactiveLandmarkAction);
}

//-----------------------------------------------------------------------------
Expand Down
Expand Up @@ -73,7 +73,7 @@ class qSlicerSubjectHierarchySegmentPluginPrivate: public QObject
void init();
public:
QAction* SegmentWithMenuAction;
QMenu* SegmentWithMenu;
QSharedPointer<QMenu> SegmentWithMenu;
};

//-----------------------------------------------------------------------------
Expand All @@ -84,7 +84,6 @@ qSlicerSubjectHierarchySegmentPluginPrivate::qSlicerSubjectHierarchySegmentPlugi
: q_ptr(&object)
{
this->SegmentWithMenuAction = NULL;
this->SegmentWithMenu = NULL;
}

//-----------------------------------------------------------------------------
Expand All @@ -111,8 +110,8 @@ void qSlicerSubjectHierarchySegmentPluginPrivate::init()
// Create top-level segment action and add it to menu
this->SegmentWithMenuAction = new QAction("Segment this using...",q);

this->SegmentWithMenu = new QMenu();
this->SegmentWithMenuAction->setMenu(this->SegmentWithMenu);
this->SegmentWithMenu = QSharedPointer<QMenu>(new QMenu());
this->SegmentWithMenuAction->setMenu(this->SegmentWithMenu.data());

QAction* editorAction = new QAction("Editor",q);
QObject::connect(editorAction, SIGNAL(triggered()), q, SLOT(segmentCurrentNodeWithEditor()));
Expand Down Expand Up @@ -167,7 +166,7 @@ QMenu* qSlicerSubjectHierarchySegmentPlugin::segmentWithMenu()
{
Q_D(qSlicerSubjectHierarchySegmentPlugin);

return d->SegmentWithMenu;
return d->SegmentWithMenu.data();
}

//---------------------------------------------------------------------------
Expand Down

0 comments on commit ae453ef

Please sign in to comment.