What should happen ist the following:
if an Interactor is removed, all references in within the Interaction Classes (e.g. Dispatcher) should be removed also.

This seems to be broken eigther at DataNode or DataInteractor level.

BindDispatcherInteractor (RegisterDataStorageEvents) listens for these changes,
and accordingly removes the Interactor, my initial gues would be that these aren't triggered.

Some more data:

I'm taking this data from a NodeRemoved callback in my View.

If I execute:

dataNode->SetDataInteractor(NULL);

the number of references to the DataNode are still 5, but the number of references to the DataInteractor is 1.

If I execute:

dataNode->SetDataNode(NULL);

the number of references to the DataNode drop to 4 and the DataInteractor destructor is called.

In none of these cases the destructor of the DataNode is called when returning from NodeRemoved().

When querying the Dispatcher with GetNumberOfInteractors(), before and after the call to SetDataInteractor(NULL)/SetDataNode(NULL), the number of interactor reported is zero.

I've collected some more information that changes a little what I've reported before.
For my tests I'm using a mitk::PointSet and a mitk::PointSetInteractor.

1. If the node containing a PointSet is added to the DataStorage and there is NO picking on a surface (in the 3D render window the scene is rotated but no surface is picked) then:

a. if the DataInteractor is set to NULL in the NodeRemoved event, the DataNode is destroyed.
b. if you don't set the DataInteractor to NULL, the DataNode is still in memory with a reference count of 1.
c. if you try to get the DataInteractor and set its DataNode to NULL, you get an access violation in the notification DataNodeChanged().

2. If, after adding the DataNode, you pick on any surface, then (this was my original report situation):

a. if the DataInteractor is set to NULL in the NodeRemoved event the DataNode is still in memory and its reference count if 1.
b. if you don't set the DataInteractor to NULL, the DataNode is still in memory with a reference count of 2.
c. if you try to get the DataInteractor and set its DataNode to NULL, you get an access violation in the notification DataNodeChanged().

To collect this data I've instrumented the Register and UnRegister virtual member functions of my DataNode and collected the call stacks and their corresponding reference counts (under GNU/Linux). If you would like to test with this code, I can share it.

I'll have a look at this later this day, if you can share some code that would be great. One more question: are you working on a current or recent master version ?

User webechr has pushed new remote branch:

bug-19329-FixMultiReference

One minor problem was in PointSetInteraction which held a reference of last selection that wasn't updated on deletion, so the PointSet was held until a new one was selected.

The main problem lay in the handling of deletion from the DataManager,
since the reference between DataInteractor and DataNode was not resolved the objects could be destroyed. To solve this two things have been implemented:

• DataInteractors have a new function to to handle if a DataNode is deleted
• Dispatcher removes Interactor that have no reference to a DataNode

What remains is to write a test, that constructs this scenario and checks that the object reference counts add up, after sth is removed from the DataStorage.

@Federico can you merge this branch and check if this resolves your issue.
It should not be neccessary that to manually handle the case when sth is deleted from the DataManager.

Enhanced mitkDispatcherTest to check if reference counters of DataNode and DataInteractor are correct after removing a DataNode with active Interactor from DataStorage.

@Christian Yes, thank you, this solves that issue.

I've been also tracking the other reference that makes the destruction of the DataNode non-deterministic when picking nodes:

In VtkPropRenderer::InitPathTraversal() a vector of smart pointers to DataNode is saved in m_PickingObjects (this member function is called when calling PickObject). That vector is never freed until it is overwritten in the next call to VtkPropRenderer::InitPathTraversal(); in some situations the reference is alive until the Workbench is closed and the VtkPropRenderer destroyed.

Do you know if it is possible to remove that reference synchronously with the DataStorage removal?

Sample code to reproduce this: https://dl.dropboxusercontent.com/u/28800175/TestProject.zip

I had a look at the MitkCore related changes. To help for the core change flag, I'll share my findings here. Please correct me if I misunderstood something:

• Changes handle a special cleanup case in mitk::Dispatcher, add a test for this case

• I think the changes stabilize the mechanism regarding the mentioned use case, should be integrated after reaction to the following point.

• Minor issue: the name of the new method DataInteractor::DeletedNode() suggests that this method handles the reaction to some DataNode having been deleted. Just seeing the code without executing tests or regarding stack traces suggests that Dispatcher is just being asked to forget about some specified DataNode, so it calls DataInteractor::DeletedNode(). Suggestion: rename "DeletedNode" to "FreeDataNodeReference" or "ForgetDataNode", adapt the comment. Alternative: could adding a "signalModified" flag (default true) to DataInteractor::SetDataNode() achieve the same result? This would get us rid of the new method that needs a name and explanation.

• Question, unrelated to the bug: it seems weird and dangerous that we hold SmartPointers in both directions between DataNode and DataInteractor. I did not find this mentioned in Doxygen's DataInteractionPage. Do you have a pointer to an explanation or a brief summary?

User kolbch has pushed new remote branch:

bug-19329-FixMultiReferenceRebase

The bug has been fixed in the recently pushed branch.

Instead of keeping a pointer to the DataNodes, we are now extracting all vtkAssemblyPaths directly in InitPathTraversal.
An implementation of vtkCollection is used for the management of the paths and initialization of iterators.
This is nearly the same procedure as in vtkAssembly.

[ab3c96]: Merge branch 'bug-19329-FixMultiReferenceRebase'

Merged commits:

2015-10-28 14:13:16 Christoph Kolb [6ac583]
make members private

2015-10-22 11:49:53 Christoph Kolb [b77b2f]
save paths in assemblycollection, so the datanode pointers are not needed.

2015-09-23 18:01:59 Christian Weber [0393fb]
extent test to check reference counts of Interactors and DataNodes

2015-09-23 16:43:50 Christian Weber [c87e61]
add a function to deal with deletion of DataNodes to DataInteractor

this was needed to have a way of removing a DataNode without triggering
modified request on the removed DataNode (as the corresponding DataInteracor
would be updated)
this function will be called when from the Dispatcher when a DataNode is removed.

2015-09-23 16:19:32 Christian Weber [628038]
Dispatcher will also remove all DataInteractor with no valid DataNode

2015-09-23 16:13:03 Christian Weber [54891c]
check for nullptr in DataNodeChange

to prevent crash when updating property lists

2015-09-23 16:12:32 Christian Weber [4f8940]
update current selection to nullptr if nothing is selected

else a reference to a potentially deleted pointset is held, preventing
it from being destroyed