Page MenuHomePhabricator
Create Task
Maniphest T3064

Avoid Qt event reentrance through QmitkProgressBar
Closed, ResolvedPublic
Actions

Description

QmitkProgressBar currently calls

qApp->ProcessEvents()

in its SlotProgress() and SlotAddStepsToDo() methods. While this somehow allows users to interact with the application while the progress bar is being updated, it can lead to reentrant calls to MITK code (in particular via state machine event processing). This can result in nested calls which can easily lead to application misbehavior.

When removing the ProcessEvents() calls, the ProgressBar still works correctly (GUI is updated), and the above problem does no longer occur.

The problem was observed in QmitkImageStatistics, where this can occasionally lead to the wrong statistics/histogram being displayed when the user interacts with the DataStorage while a previous statistics calculation is still running.

Attached is a list of files which include the ProgressBar. The corresponding bundles should be tested when removing the lines.

FilesUsingProgressBar.txt8 KBDownload

Event Timeline

seitelm added projects: MITK, Bugzilla.Nov 20 2009, 1:47 PM
maleike added a subscriber: maleike.Nov 20 2009, 2:38 PM

These are more than 100 files. Let's assume that at least half of them affect 3M3. Could a workaround prevent image statistics recalculation?

I agree that recursive calls to the event loop are a not good and this behavior should be avoided by design. However, testing this many classes seems impossible in one afternoon.

nolden added a subscriber: nolden.Nov 23 2009, 1:24 PM

I reduced the list to 3M / Liver related classes

FilesUsingProgressBar.txt2 KBDownload

seitelm added a subscriber: seitelm.Nov 25 2009, 4:47 PM

[SVN revision 20548]
FIX (#3064): Prevent reentrant calls to UpdateStatistics() invoked by Qt piped update requests

seitelm added a comment.Nov 25 2009, 4:55 PM

(In reply to comment #3)

The above fix is a workaround for the initial problem in QmitkImageStatistics; the issue does not occur any more even with the current QmitkProgressBar behavior.

My preferred solution would still be to change the QmitkProgressBar behavior. However, when changing the current behavior to not allow processEvents() calls any more, many modules will simply not react any more while their time consuming algorithms are executed (external threads will probably be the only clean solution).

Removing 3M3 keyword

@Marco: is that okay? Otherwise, please reassign.

seitelm added a comment.Jan 13 2010, 3:59 PM

The reentrance problem was observed in QmitkImageStatistics, which is not (yet) open source.

It may however occur also in other bundles using QmitkProgressBar.

When modifiying QmitkProgressBar as proposed, all affected bundles would need some extra testing.

Relevant for MITK 1.0?

seitelm added a comment.Jan 27 2010, 5:13 PM

[SVN revision 21136]
FIX (#3064): Revert changes of r20548 (workaround for QmitkProgressBar re-entrance problem); problem will be solved globally

seitelm added a comment.Jan 28 2010, 9:20 AM

[SVN revision 21145]
FIX (#3064): Replace explicit processing of Qt process loop by immediate update of render windows to avoid re-entrance issues

seitelm added a comment.Jan 28 2010, 9:22 AM

Tested for a couple of applications:

  • Non-threaded applications do block now while progress bar is counting up (but rendered views are still updated if requested during processing)
  • Threaded applications are still interactive
  • Event loop re-entrance does no longer occur from QmitkProgressBar

Closing bug.

MITK · MIC · IMSY · DKFZ