call stack when crashing on exit

Looks like a LayerController or static cleanup problem. Does it still happen?

Still happening...

I can reproduce the crash on the current git master. I will take a look at it.

We seem to have identified the problem:

The crash is caused by the destructor of a static variable (the UndoController) destroying the static s_LayerControllerMap before it is correctly removed by the vtkLayerController.

The entire thing is based in the multitude of static singletons used in MITK, whose initialization and destruction order is randomly determined. While this can work, it does not have to.

Several different possibilities exist for solving this.

1. Removing all static singletons and replacing them by something different
   
   While very desirable this is by far the most work intensive solution and it is not entirely sure whether it is at all feasible.

2. One super singleton managing all the other singletons
   
   Would work, but is not a beautiful solution.

3. Creating a central service started upon loading the dll which administers the singletons
   
   This is similar to the super singleton solution in its feasability but a better way of doing it.

A way to do this would be to create a static Init() function which starts the service. The latter in turn would register all the known singletons and at program end deregister them.

This would require developers to use enter their modules (if they require a singleton) into the service. An easier way of doing this might be by creating a function which allows for self registration at the service, which in turn will continue administration from then on.

Removing RFD for now.

Sascha, didn't you want to draft an implementation?

I try to recap my analysis for the specific problem.

In short, the classes in question are not proper singletons, but use static member variables. These get deleted at "random" order during unloading of the shared library. One of those static members is a map holding (through a chain of aggregated members) smart pointers to mitk::BaseRenderer instances. The destructor of mitk::BaseRenderer calls a static method which tries to access another static member (in mitk::VtkLayerController) which was already destroyed.

The classes and static members causing the trouble are

• mitk::VtkLayerController : member "static std::map<...> s_LayerControllerMap"
• mitk::UndoController: member "static UndoModel::Pointer m_CurUndoModel"

Now assume that on application shut-down, during unloading of the Mitk shared library, the static member s_LayerControllerMap is destroyed before m_CurUndoModel.

The static m_CurUndoModel smart pointer holds a mitk::LimitedLinearUndo pointer which in the end references mitk::DisplayCoordinateOperation smart pointers. This class holds a smart pointer to mitk::BaseRenderer which calls the mitk::BaseRenderer destructor during its own destruction when the static m_CurUndoeModel smart pointer is destroyed. Now the mitk::BaseRenderer destructor calls the static mitk::VtkLayerController::RemoveInstance(...) method which tries to access the already deleted static s_LayerControllerMap member. This causes the crash.

Recap:

• There are no classic singletons involved
• Using static members in destructors (directly or indirectly) is bad
• Holding on to objects via smart-pointers too long is bad
• Granting access to static members (directly or via static member functions) is bad

Solutions:

• Use a mitk::WeakPointer (*not* a itk::WeakPointer) mitk::DisplayCoordinateOperation to hold on to a mitk::BaseRenderer. This might work, but could just shift the problem to another smart pointer member. Anyway "operations" should probably not keep mitk::BaseRenderer objects alive.

• Redesign mitk::VtkLayerController to work without statics and put the control of deletion order in the hands of the programmer.

Setting Caspar as assignee, since he can reproduce this bug.

Please implement and test solution one (mitk::WeakPointer) from comment #7 and close the bug if this solves the problem.

This needs a core modification flag, the bug branch has bee pushed.

[e4c908]: Merge branch 'bug-4733-hold-BaseRenderer-As-Weak-Pointer'

Merged commits:

2011-11-16 14:42:46 Caspar Goch [a2b153]
Switched from Smart to Weakpointer