Can also cause a crash on scene close when saved a scene with fiducials in it, as the annotation hierarchy node lost its display node on scene save.

Looks like the culprit is in the save data dialog when it calls:
// remove unreferenced nodes
vtkMRMLLogic *mrmlLogic = vtkMRMLLogic::New();
mrmlLogic->SetScene(this->MRMLScene);
mrmlLogic->RemoveUnreferencedDisplayNodes();
mrmlLogic->RemoveUnreferencedStorageNodes();
mrmlLogic->Delete();

In RemoveUnreferencedDisplayNodes, it goes through the displayable nodes and makes sure each display node is pointed to by something. The problem is that the hierarchy nodes aren't displayable nodes, so their display nodes get removed.
The call to RemoveUnreferencedDisplayNodes was added in svn 15936 in Jan 2011, before I added the saving of a default scene view on mrml scene save.
Something else in the scene node accounting must have changed to cause this crash on scene close (the qt scene model hits an assert that a node was removed without a notification).
When I remove this logic call, the display node doesn't disappear, but I still get the "node removed without notification crash" on scene close. That call is only used in the save data dialog, so there's something else clearing out display nodes that aren't pointed to by displayable nodes (it may be that the wrong Reference macro is used so that the mrml scene doesn't have it properly recorded in the list of referenced nodes).

Alex: what's the best vtkMRMLScene method to call to trigger a clean up of unreferenced nodes after saving a mrml scene?

More digging: if I keep the mrmlLogic calls out, creating a model hierarchy and then saving it, the hierarchy display node is fine in the scene references:
h1 = slicer.mrmlScene.GetNodeByID("vtkMRMLModelHierarchyNode1")
m = slicer.mrmlScene.GetNodeByID("vtkMRMLModelNode5")
col = slicer.mrmlScene.GetReferencedNodes(h1)
col.GetItemAsObject(1).GetID()
returns model display node 6 (0 returns itself)
And if check the model node after save:
col = slicer.mrmlScene.GetReferencedNodes(m)

col.GetItemAsObject(0).GetID()
'vtkMRMLModelNode5'
col.GetItemAsObject(1).GetID()
'vtkMRMLModelStorageNode1'
col.GetItemAsObject(2).GetID()
'vtkMRMLModelDisplayNode5'
t = slicer.mrmlScene.GetNodeByID("vtkMRMLModelDisplayNode5")
t.GetID()
'vtkMRMLModelDisplayNode5'

but when close the scene, it crashes with this error:
ERROR: In /projects/birn/nicole/Slicer4/Slicer4/Libs/MRML/Core/vtkMRMLScene.cxx, line 1356
vtkMRMLScene (0x1510680): RemoveNode: Node vtkMRMLModelDisplayNode/ModelDisplay_4[0x7159fc0] already removed

ERROR: In /projects/birn/nicole/Slicer4/Slicer4/Libs/MRML/Core/vtkMRMLScene.cxx, line 1364
vtkMRMLScene (0x1510680): RemoveNode: class: vtkMRMLModelDisplayNode name:ModelDisplay_4 id: vtkMRMLModelDisplayNode5[0x7159fc0] can't be found by ID

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 /projects/birn/nicole/Slicer4/Slicer4/Libs/MRML/Widgets/qMRMLSceneModel.cxx, line 1131

Is something in the scene close removing nodes in the wrong order?

It could be that something's watching for node removed events and removing related nodes before the scene RemoveAllNodesExceptSingletons gets to them in it's loop. Putting
this->Start/EndState(vtkMRMLScene::BatchProcessState);
around the loop didn't help. But in vtkSlicerModelsLogic.cxx if I turned off AutoRemoveDisplayAndStorageNodes, no crash on scene close. That method doesn't currently check for scene state, is adding a check there for the mrml scene being in a close scene state enough?

Did a quick test of checking for scene IsClosing, no crash.

The combo of two methods trying to clean up the scene caused two interelated crashes, my recommendation is:

• take out the clean up from the save dialog
• add checks for IsClosing and/or BatchProcessing in modules that implement OnMRMLSceneNodeRemoved
• make sure that modules that add extra nodes when nodes are added clean up after themselves when those nodes are removed

svn 19519 adds a check for IsClosign to the models OnMRMLSceneNodeRemoved

svn 19525 removes the clearing out of display and storage nodes on scene save, need to make the modules respond to node removed events to clear out "extra" nodes that they've added (display, storage) for the nodes they manage.