Page MenuHomePhabricator

Reinit and or DataStorage interaction
Closed, ResolvedPublic

Description

MITK crashes, when images are generated and added to the DataStorage using the DataNodeFactory. Image clone and image copy are also utilized on an binary image node.

The crash occurs after some visibility toggling or reinit, global reinit on DataNodes in the DataStorage.

Event Timeline

Update:
The crash already happens on normal toggle to visible and reinit on the loaded data node. The example file includes a minimal plugin with the error. Also the call stack as well as the mitk log are provided.

I have very similar problems, the program crashes quite often doing a global reinit after adding a new node.

According to Matthias the crash has most likely the following reason:

  • Reinit forces a crosshair update on the toplayer node
  • if a node is added with datastorage->Add(node, parent) and parent is NULL then the toplayer node is also NULL. Hence mitk crashes.

Solution might be to check the top layer node for null

Hi Matthias, I am not able to reproduce the crash. I tested your plugin on the current master as well as on the 2013.06.00 release. I started the workbench and activated your plugin. Pressing the button added the image to the data storage. I was able to play around with the visibility and the reinit/global reinit without any crash.

Maybe we need some more details:

Do you have some special configuration?
Which build type do you use (release, debug)?
Which compiler do you use? (I am using the clang compiler)
On which operating system are you working? (I tested it on Mac 10.7)

After your post I tested some more things. As it seems the crash occurs, when I load a binary image segmentation in the .nrrd format. Using a normal grey scale image, all init or reinit functionality works fine using the following Add function of DataStorage:

GetDataStorage()->Add(node2, (mitk::DataNode*)nullptr);

With the binary .nrrd I need to change the line above to
GetDataStorage()->Add(node2);//, (mitk::DataNode*)nullptr);
to prevent the crash happening on reinit or InitializeViews.

Was by any chance something changed in the .nrrd format or the reader so that the node is somehow corrupted after generation with the DataNodeFactory? I could provide the image files I'm using in the example if this would help.

To your questions:

Do you have some special configuration?
-I'm not sure what you mean by this.
The MITK build was performed without superbuild using the components itk 4.4.0, vtk 5.10.1, qt 4.8.5.

Which build type do you use (release, debug)?
-The crash occurs with all build types.

Which compiler do you use? (I am using the clang compiler)
-I use the Visual Studio 10 x64 copiler

On which operating system are you working? (I tested it on Mac 10.7)
-Windows 7

This is now part of the release-cycle SCRUM Program.

New remote branch pushed: bug-15900-ReinitAndOrDataStorageInteraction

While i could not strictly reproduce the bug, I think I found the source. Calling the convenience method Add(Node data, Node parent) with a null parent caused the routine to create an invalid parent set that was then passed on anc could cause a crash later on. If the release flag is set, I will merge my changes, which I expect to solve the problem.

(In reply to Keno März from comment #8)

While i could not strictly reproduce the bug, I think I found the source.
Calling the convenience method Add(Node data, Node parent) with a null
parent caused the routine to create an invalid parent set that was then
passed on anc could cause a crash later on. If the release flag is set, I
will merge my changes, which I expect to solve the problem.

I agree that's the most likely cause of the problem cf. comment #3. But it's rather some faulty or empty geometry on node reint or view update. Especially since functions like QmitkStdMultiWidget::GetTopLayerNode and
QmitkStdMultiWidget::HandleCrosshairPositionEventDelayed involve the parent (TopLayerNode?).

One should probably discuss if a null parent makes sense from this point of view. Do you explicitly need a null node as a parent? Many function access this node and expect the parent to be a valid node, which will probably cause problems in various locations further down the road.

If you require such functionality, I will make this a topic in our next MITK-Meeting

I also require a core Modification Flag for this. Changes are trivial (just a check for null and a reroute of this call), so I hope we don't need a full request.

(In reply to Keno März from comment #10)

I also require a core Modification Flag for this. Changes are trivial (just
a check for null and a reroute of this call), so I hope we don't need a full
request.

Keno, I tried to read up a bit on the long discussion but didn't manage to quickly make sense of "reroute of this call"..

Could you perhaps quickly summarize the intended change? You don't need a wiki page, but at least: where do you want to change what?

Hey Daniel,

thanks for getting back to us so quickly!

In mitkDataStorage.cpp I added the line with the comment. Thus, when the method is called with a null parent, the an empty set is handed to the standard add method which can handle this case correctly. Otherwise, the parent set would contain a null node which would cause a NullPointerException later on.

void mitk::DataStorage::Add(mitk::DataNode* node, mitk::DataNode* parent)
{

mitk::DataStorage::SetOfObjects::Pointer parents = mitk::DataStorage::SetOfObjects::New();
if (parent != NULL) //< Return empty set if parent is null
  parents->InsertElement(0, parent);
this->Add(node, parents);

}

[501df7]: Merge branch 'bug-15900-ReinitAndOrDataStorageInteraction'

Merged commits:

2013-09-11 16:07:31 Keno Maerz [6b1d53]
correctly rerouting call for null parent