View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0002622 | Slicer4 | Core: GUI | public | 2012-10-06 14:18 | 2014-03-06 05:01 |
Reporter | pieper | Assigned To | jcfr | ||
Priority | high | Severity | crash | Reproducibility | have not tried |
Status | closed | Resolution | unable to reproduce | ||
Product Version | |||||
Target Version | Slicer 4.3.1 | Fixed in Version | Slicer 4.2.0 | ||
Summary | 0002622: assert crash in scene model closing scene after change tracker | ||||
Description | The change tracker tutorial all runs fine until the last step of closing the scene. At this point the following is printed and the stack trace pasted below is triggered. Close Scene ERROR: In /Users/pieper/slicer4/latest/Slicer/Libs/MRML/Core/vtkMRMLScene.cxx, line 1391 ASSERT failure in qMRMLSceneModel::onMRMLSceneNodeAboutToBeRemoved(): "A node has been removed from the scene but the scene model has never been notified it has been added in the first place. Maybe vtkMRMLScene::AddNodeNoNotify() has been used instead of vtkMRMLScene::AddNode", file /Users/pieper/slicer4/latest/Slicer/Libs/MRML/Widgets/qMRMLSceneModel.cxx, line 1117 | ||||
Steps To Reproduce | As of r21126 there is an RSNA2012Quant self test which triggers this. Select the change tacker tutorial and the crash happens at the end. Note that it is actually after the volume rendering is added that the crash on scene close occurs, so if you want to reproduce more quickly you could skip the registration and analysis steps. Note that this patch must be applied for the self test to work: | ||||
Additional Information | Thread 0 Crashed:: Dispatch queue: com.apple.main-thread | ||||
Tags | No tags attached. | ||||
related to | 0002621 | closed | finetjul | Switching to VR after manually creating display node causes crash |
related to | 0002713 | acknowledged | jcfr | vtkMRMLNode::SetScene() should be protected and set by scene |
related to | 0003432 | closed | nicole | Running RSNA2012Quant test compains about missing nuclear medicine output csv parameter file |
child of | 0002517 | closed | pieper | Classroom Course 2 |
Most likely same problem as what is described in comment 0006428 in issue 0002621 |
|
After updating to r21137 I now get this crash... (Reproduce this by going to Modules->Testing->TestCases->RSNA2012Quant and running the ChangeTracker test) Thread 0 Crashed:: Dispatch queue: com.apple.main-thread |
|
I tested this after the previous change of Julien, and the test ran for me without crash including the scene close. Will update and test again. |
|
Crash is reproducible outside ChangeTracker as follows: 1) download brain tumor 1 from sample data vrNode = slicer.modules.volumerendering.logic().CreateVolumeRenderingDisplayNode() rs = slicer.mrmlScene.GetNodesByClass('vtkMRMLAnnotationROINode') vrLogic = slicer.modules.volumerendering.logic() if propNode == None: vrNode.SetAndObserveVolumePropertyNodeID(propNode.GetID()) Julien: I tested this with r21138. I dereference the node, and add manually to the scene. Am I doing something incorrectly? Is this a VR issue? |
|
In that script the display node is not added into the displayable node. (see comment 0006442 in 0002621). |
|
Ok, script updated as you suggested, but the crash is the same. vrNode = slicer.modules.volumerendering.logic().CreateVolumeRenderingDisplayNode() vrNode.SetReferenceCount(vrNode.GetReferenceCount()-1)print(str(vrNode.GetReferenceCount())) rs = slicer.mrmlScene.GetNodesByClass('vtkMRMLAnnotationROINode') vrLogic = slicer.modules.volumerendering.logic() if propNode == None: vrNode.SetAndObserveVolumePropertyNodeID(propNode.GetID()) |
|
Crash fixed in r21141 The root problem is that the vrNode is added in the scene after CopyDisplayToVolumeRenderingDisplayNode() . But CopyDisplayToVolumeRenderingDisplayNode() needs a pointer to the volume node v and vrNode can't return any reference pointers if it is not added to the scene first. So I would suggest instead: |
|
Julien, thanks, no crash anymore! Now if I run this same script: vrNode = slicer.modules.volumerendering.logic().CreateVolumeRenderingDisplayNode() vrNode.SetReferenceCount(vrNode.GetReferenceCount()-1)print(str(vrNode.GetReferenceCount())) rs = slicer.mrmlScene.GetNodesByClass('vtkMRMLAnnotationROINode') vrLogic = slicer.modules.volumerendering.logic() if propNode == None: vrNode.SetAndObserveVolumePropertyNodeID(propNode.GetID()) I have the following memory leaks, as you see display node is not deallocated: Class "vtkCellData" has 9 instances still around. ---> Class "vtkMRMLCPURayCastVolumeRenderingDisplayNode" has 1 instance still around. Class "vtkMRMLSliceNode" has 3 instances still around. |
|
The memory leak is a duplicate of 0002621 |
|
No crash running RSNA quantitative test as of r21143 |
|
From the hangout: Julien Finet Julien Finet |
|
Andrey - Can you try the changes that Julien suggested? |
|
Julien says you need to initialize the ROI node... |
|
The following should help: roi = slicer.vtkMRMLAnnotationROINode() (note that the ChangeTracker documentation is not up to date, it links to 4.0) |
|
Which version are you looking at? CreateNodeByClass has been fixed a while ago, and Initialize() was added yesterday. References for GetNodesByClass are also decremented. Current version of ChangeTrackerPy: https://github.com/fedorov/ChangeTrackerPy/commit/b4fa890c98e162ee51394a52f6577fc1a4eed7a5 Version in External_ChangeTracker: b4fa890c98e162ee51394a52f6577fc1a4eed7a5 which version of the code are you looking at? |
|
I was looking at 68b70e15ada3b46742ab3374a62e3eb02a5d101a, my bad. vtkMRMLAnnotationNode::Initialize adds the node into the scene, so you shouldn't have to add it again after. |
|
fixed in r21291. Would be great if there was a uniform way to add nodes to the scene. FYI, I did not have any crashes (neither on my build, nor on nightly linux). And why can't AddNode() be a no-op if the node is already in the scene. |
|
|
|
Re crash: my bad, I didn't check the dashboard! The test was not crashing on my machine. About AddNode(): vtkMRMLNode keeps pointer to the scene, so can't we just check the pointers in AddNode to figure out if the node is in the scene in question or not? |
|
|
|
Makes sense. Added issue 0002713 to keep track of this last point. |
|
Has this been resolved ? I would like to confidently resolve issue 0002517 and start the release process. |
|
No, not resolved. This still happens on my latest build: http://slicer.cdash.org/testDetails.php?test=3084373&build=45566 But since this is an assert, it only happens on debug builds and may not be an issue for the tutorials. I am testing a windows release build to see if it crashes at exit. |
|
I tested the windows release build and had no problems. Since this is the version that will be used for the training, I don't think this issue should be considered a roadblock (I would still like to see it resolved though). |
|
Thanks for checking. Targeting for 4.2.1 and marking "Classroom Course 2" has resolved. |
|
Andriy> Are you working on resolving the root cause of the assert ? |
|
JC> No, I didn't realize this was the expectation. Is the root cause of the assert in ChangeTracker module? I thought it is something in Annotations related to ROI initialization. I thought I did all the fixes that Julien suggested, so the ROI node should be initialized correctly. Also, the test is passing on the nightly dashboard, since I think Steve mentioned it is reproducible only in Debug mode. Are we going to set up Debug dashboards to track this issue? |
|
The issue related to the tutorial has been closed. We are not addressing the "real" cause of the problem causing the issue which is also depicted by the title of the present issue "assert crash". Considering the fact the issue was assigned to you, I naturally asked for an update :) Based on your feedback, re-assigning the issue seems to be the next things to do. That said, would you have time to investigate and try to fix the problem ? There are currently no Debug dashboard, this is something we could do when the "factory-south" will be up and running. |
|
JC, I can try, but would really rather spend my Slicer development time on Reporting module. And I really don't have time (as everyone else, of course). I was also wondering if it is wise for me to debug this, rather than some of the developers familiar with Annotations module. I guess to start I will need to compile Slicer in Debug mode and check if I can reproduce the issue on my mac. |
|
Also, looking at the issue history, it was originally assigned to Julien, then in note 0006933 reassigned to me based on Julien's feedback that I was not doing things correctly. I fixed the items Julien asked me to fix, but the issue never got reassigned to Julien. |
|
After building locally the ChangeTrackerPy module and adding the following paths to the list of "additional module paths" (Menu -> Edit -> App Settings -> Module ..): /path/to/ChangeTrackerPy-build/lib/Slicer-4.2/cli-modules Try to run the part3 of the "RSNA2012Quant" self test returned the following error: unpacking to /tmp/__BundleLoadTemp2013-09-03_15+09+30.792 Configure Module |
|
Reminder sent to: fedorov @fedorov: Would it be possible to fix the failing self tests (see note 9848), that away we could try to fix the crash for the 4.3.1 release. Thks |
|
@jcfr: I was not aware of this, sorry. Looks like CTK API changed. I will look into this. |
|
@andriy: You not being aware aware of the problem can be explained by the fact the self test "RSNA2012Quant" (when run every night) checks locally for the availability of the ChangeTracker. Since the associated modules are not loaded, the self tests skips it. Thanks for looking into this |
|
@jcfr: well, even if the self-test was failing, I am not checking the dashboard regularly and would not notice... Patched version: https://github.com/Slicer/ExtensionsIndex/pull/418 Note the self test does not crash, at least not on my Mac. |
|
Unable to reproduce running RSNA2012Quant test using Slicer r22597 built in Debug/64-bit on Ubuntu 13.04 |
|
Closing resolved issues that have not been updated in more than 3 months |
|
Date Modified | Username | Field | Change |
---|---|---|---|
2012-10-06 14:18 | pieper | New Issue | |
2012-10-06 14:18 | pieper | Status | new => assigned |
2012-10-06 14:18 | pieper | Assigned To | => finetjul |
2012-10-06 14:19 | pieper | Relationship added | child of 0002517 |
2012-10-07 19:44 | finetjul | Relationship added | related to 0002621 |
2012-10-07 20:08 | finetjul | Assigned To | finetjul => fedorov |
2012-10-07 20:09 | finetjul | Note Added: 0006429 | |
2012-10-09 04:40 | pieper | Note Added: 0006444 | |
2012-10-09 04:41 | pieper | Note Edited: 0006444 | |
2012-10-09 05:14 | fedorov | Note Added: 0006446 | |
2012-10-09 05:29 | fedorov | Note Added: 0006448 | |
2012-10-09 05:29 | fedorov | Assigned To | fedorov => finetjul |
2012-10-09 05:34 | finetjul | Note Added: 0006449 | |
2012-10-09 05:38 | fedorov | Note Added: 0006450 | |
2012-10-09 06:02 | finetjul | Note Added: 0006452 | |
2012-10-09 06:02 | finetjul | Status | assigned => resolved |
2012-10-09 06:02 | finetjul | Fixed in Version | => Slicer 4.2.0 - coming release |
2012-10-09 06:02 | finetjul | Resolution | open => fixed |
2012-10-09 06:14 | fedorov | Note Added: 0006453 | |
2012-10-09 06:14 | fedorov | Status | resolved => feedback |
2012-10-09 06:14 | fedorov | Resolution | fixed => reopened |
2012-10-09 06:18 | finetjul | Note Added: 0006455 | |
2012-10-09 06:52 | fedorov | Note Added: 0006459 | |
2012-10-09 06:52 | fedorov | Status | feedback => resolved |
2012-10-09 06:52 | fedorov | Resolution | reopened => fixed |
2012-10-30 12:23 | pieper | Note Added: 0006933 | |
2012-10-30 12:26 | pieper | Status | resolved => assigned |
2012-10-30 12:26 | pieper | Assigned To | finetjul => fedorov |
2012-10-30 12:27 | pieper | Note Added: 0006934 | |
2012-10-30 12:27 | pieper | Status | assigned => feedback |
2012-10-30 12:30 | pieper | Note Added: 0006936 | |
2012-10-30 12:36 | finetjul | Note Added: 0006939 | |
2012-10-30 15:30 | fedorov | Note Added: 0006949 | |
2012-10-30 17:54 | finetjul | Note Added: 0006955 | |
2012-10-30 18:22 | fedorov | Note Added: 0006957 | |
2012-10-30 19:20 | finetjul | Note Added: 0006958 | |
2012-10-31 04:20 | fedorov | Note Added: 0006959 | |
2012-10-31 04:36 | finetjul | Note Added: 0006960 | |
2012-10-31 05:48 | fedorov | Note Added: 0006963 | |
2012-10-31 05:49 | fedorov | Relationship added | related to 0002713 |
2012-10-31 11:38 | jcfr | Note Added: 0006986 | |
2012-10-31 11:51 | pieper | Note Added: 0006988 | |
2012-10-31 11:54 | pieper | Note Added: 0006989 | |
2012-10-31 11:55 | jcfr | Note Added: 0006990 | |
2012-10-31 11:56 | jcfr | Note Edited: 0006990 | |
2012-10-31 11:56 | jcfr | Target Version | Slicer 4.2.0 - coming release => Slicer 4.2.1 |
2012-10-31 11:56 | jcfr | Additional Information Updated | |
2012-11-06 10:00 | jcfr | Note Added: 0007079 | |
2012-11-06 11:53 | fedorov | Note Added: 0007085 | |
2012-11-06 14:23 | jcfr | Note Added: 0007089 | |
2012-11-06 15:10 | fedorov | Note Added: 0007090 | |
2012-11-06 15:12 | fedorov | Note Added: 0007091 | |
2012-11-07 04:51 | jcfr | Status | feedback => assigned |
2012-11-07 04:51 | jcfr | Assigned To | fedorov => finetjul |
2012-11-09 03:19 | jcfr | Assigned To | finetjul => jcfr |
2012-11-16 12:01 | jcfr | Target Version | Slicer 4.2.1 => Slicer 4.2.2 |
2012-12-07 17:59 | jcfr | Target Version | Slicer 4.2.2 => Slicer 4.2.3 |
2013-02-12 09:37 | jcfr | Target Version | Slicer 4.2.3 => Slicer 4.3.0 |
2013-09-03 11:15 | jcfr | Target Version | Slicer 4.3.0 => Slicer 4.3.1 |
2013-09-03 11:18 | jcfr | Note Added: 0009848 | |
2013-09-03 11:18 | jcfr | Status | assigned => feedback |
2013-09-03 11:19 | jcfr | Note Added: 0009849 | |
2013-09-03 11:25 | fedorov | Note Added: 0009850 | |
2013-09-03 11:33 | jcfr | Note Added: 0009852 | |
2013-09-03 12:10 | fedorov | Note Added: 0009856 | |
2013-10-04 00:47 | jcfr | Note Added: 0010130 | |
2013-10-04 00:47 | jcfr | Status | feedback => resolved |
2013-10-04 00:47 | jcfr | Resolution | fixed => unable to reproduce |
2013-10-04 00:49 | jcfr | Relationship added | related to 0003432 |
2014-03-06 05:00 | jcfr | Note Added: 0010878 | |
2014-03-06 05:01 | jcfr | Status | resolved => closed |