Fixed crash for single time step segmentations
Needs Verification750daa0b6d2b
Actions

Description

Fixed crash for single time step segmentations

Details

Auditors

Provenance

• kausch	Authored on Apr 26 2019, 11:40 AM
• kausch	Pushed on Apr 26 2019, 11:40 AM

Parents

rMITK55cb5498ee4d: Added Try-Catch For Multiple Timepoints Segmentation

Branches

Unknown

Tags

Unknown

Event Timeline

• kausch committed rMITK750daa0b6d2b: Fixed crash for single time step segmentations (authored by • kausch).Apr 26 2019, 11:40 AM

The fix is not good and has several problems. See comment above. If it is not clear how to handle it properly, just drop by an I can explaine.

/Modules/ImageStatistics/mitkImageStatisticsContainer.cpp
129–130	You cannot just create a container on the fly on the heap and pass it back as a reference. This is highly likely to cause crashes due to corrupted opject states. Don't do that. The correct way to handle this with the current API is to either caller uses try catch arround GetStatisticsForTimeStep or he checks with TimeStepExists() befor retrieving the time step. If the time step does not exist it is job of the caller to specify the fallback strategy. It is not part of the container to specify or enforce anything in this direction.

This commit now has outstanding concerns.Apr 26 2019, 1:17 PM

floca added inline comments.Apr 26 2019, 10:31 PM

/Modules/ImageStatistics/mitkImageStatisticsContainer.cpp
129–130	One correction. It will not cause crash (that would be the case with smartpointers as raw pointers. Currently it will generate memory leaks.

kalali added a subscriber: kalali.May 2 2019, 12:09 PM

kalali added inline comments.

/Modules/ImageStatistics/mitkImageStatisticsContainer.cpp
129–130	I second that. I also ran into this problem when using the classes for my own plugin. For testing I only added a mask at one time step -> crash. Checking in any way if 'GetStatisticsForTimeStep' is successfully without directly accessing 'm_Histogram' seems to be the correct way to go. You did this before with your try-catch approach. In the corresponding task you mentioned another problem (another exception?). Did you find out more about this or why did this lead to you discarding the try-catch approach?

There seems to be another exception thrown when willing the table view. The statistics table is not filled. The idea is now not to change the GetStatisticsForTimeStep functionality but handle the exception in the level above (like we started before with the try-catch)

We reverted the changes according to your concerns. Instead of creating a new ImageStatisticsObject in the GetStatisticsForTimeStep method, we now check whether the time step exists before calling the method. That fixed the problem. Statistics are now only shown for the time steps that have a segmentation associated with.

This commit now requires verification by auditors.May 10 2019, 11:29 AM