Opened 9 months ago

Last modified 4 months ago

#15799 portneeded defect (port to stable)

Pending events unexpectedly dispatched from wxGenericProgressDialog::Update() in wxGTK and MSW

Reported by: hagen Owned by:
Priority: normal Milestone: 3.0.2
Component: wxGTK Version: 3.0.0
Keywords: wxGenericProgressDialog wxGUIEventLoop YieldFor regression Cc:
Blocked By: Blocking:
Patch: no

Description

Hi,

I am using wxGTK v3.0.0 on a Ubuntu 13.10 system.

My application uses a wxThread to implement a background job. This thread produces two types of events,

  1. Progress event (wxCommandEvent)
  2. Finished event (wxCommandEvent)

These events are handled in the main thread doing the following:

  1. The progress event updates a wxProgressDialog using wxGenericProgressDialog::Update
  2. The finished event destroys the wxProgressDialog

All progress events are generated consecutively and are followed by one finished event. Thus I expect all progress events are completely handled before the finished event is handled.

Instead what I see is that the finished event is somehow processed before the last progress event is completely handled. This results in segmentation fault since the progress dialog is destroyed before the last progress event is completely handled.

The source of the problem is the call to wxProgressDialog::update which in turn calls wxGenericProgressDialog::DoBeforeUpdate which in turn calls wxGUIEventLoop::YieldFor. In wxGenericProgressDialog (line 543) it is pointed out that the call to YieldFor should only process wxEVT_CATEGORY_UI and wxEVT_CATEGORY_USER_INPUT events.
But within gtk/wxGUIEventLoop::YieldFor (line 399) the following is called: wxTheApp->ProcessPendingEvents(). This leads to the processing of all events regardless of their specific type (including my finished event).

I dug a little deeper into the problem. The last change to gtk/wxGUIEventLoop.cpp was done by Rob Bresalier between wxWidgets v2.9.4 and v2.9.5. So I compiled my application against wxWidgets v2.9.4 and v2.9.5 with the follwing result,

  1. wxWidgets v2.9.4: my application runs without problems
  2. wxWidgets v2.9.5: my application crashes due to the described problem

The diff of the two versions (2.9.4 and 2.9.5) of gtk/wxGUIEventLoop.cpp revealed that among other things the line wxTheApp->ProcessPendingEvents() (line 398/399) was added. This line in fact leads to the described problem.

At this point I can not give further information or point out a good solution to this problem due to my lack of knowledge in gtk.
Again the problem is only existing in wxGTK. I did not encounter he problem in wxMSW. I did not check against cocoa.

Change History (8)

comment:1 Changed 9 months ago by vadz

  • Keywords regression added
  • Status changed from new to confirmed
  • Summary changed from wrong event processing in wxGenericProgressDialog::Update / wxGUIEventLoop::YieldFor to Pending events unexpectedly dispatched from wxGenericProgressDialog::Update() in wxGTK

The call to ProcessPendingEvents() was added in r72723 to fix #14760 which was itself a regression. But apparently the fix was too broad and we probably should only call it if eventsToProcess == wxEVT_CATEGORY_ALL which would prevent the pending events from being dispatched from inside wxYield().

What bothers me is that I don't understand why doesn't the same problem occur in wxMSW where the code is exactly the same, i.e. calls ProcessPendingEvents() unconditionally from wxGUIEventLoop::YieldFor(). As I don't have any way to reproduce the problem and as it's probably not simple for you to make one (if it is, please do though), could you please try to debug what's going on there and why aren't the pending events dispatched under MSW?

If you could also please test that the trivial change checking for eventsToProcess == wxEVT_CATEGORY_ALL before calling ProcessPendingEvents() fixes the problem for your program, it would be very helpful too.

TIA!

comment:2 Changed 9 months ago by hagen

Hi vadz,

I added 'eventsToProcess == wxEVT_CATEGORY_ALL' to line 398 in src/gtk/evtloop.cpp.

This fixes the problem.

One side note: Before I just commented the call to ProcessPendingEvents() (line 398/399) which fixed the problem, too. But I think that's the wrong way to fix it.

comment:3 Changed 9 months ago by hagen

Hi vadz,

in wxMSW the native progress dialog (src/msw/progdlg.cpp) is used instead of the generic one. Thus completely different source code is processed.

I assume when using the generic progress dialog under wxMSW the problem would occur, too.

I hope this helps to solve the problem.

comment:4 Changed 7 months ago by VZ

(In [76060]) Don't dispatch pending events from selective YieldFor().

Pending events list can contain events from all kinds and dispatching them
from YieldFor() called to dispatch the events of some particular kind only
(e.g. to redraw the window) is unexpected and wrong, e.g. it breaks some uses
of wxProgressDialog, see #15799.

comment:5 Changed 7 months ago by vadz

  • Milestone set to 3.0.2
  • Resolution set to port to stable
  • Status changed from confirmed to portneeded

If no problems are found with the above, it should be backported to 3.0.

comment:6 Changed 4 months ago by hagen

Hi vadz,

I had the opportunity to test my code using wxGenericProgressDialog on Windows (MSW build), too. I already applied your patch r76060 to wxWidgets/trunk/src/msw/evtloop.cpp.

My first debugging sessions revealed that the wxCommandEvents are also unexpectedly dispatched. It does not result in a segmentation fault as on Linux. It just leads to the problem that my thread finished event is completely absorbed on Windows XP. The same code works on Windows7 without absorbing my thread finished event.

I compiled the wxWidgets project on Windows7 64 bit (Visual Studio 2012) for 32bit Windows XP.

Last edited 4 months ago by hagen (previous) (diff)

comment:7 Changed 4 months ago by hagen

  • Summary changed from Pending events unexpectedly dispatched from wxGenericProgressDialog::Update() in wxGTK to Pending events unexpectedly dispatched from wxGenericProgressDialog::Update() in wxGTK and MSW

comment:8 Changed 4 months ago by hagen

The problem seems to be a race condition related with the wxThread I am using on Windows (MSW build).

ATTENTION: The problem only occurred on WindowsXP AND the problem also occurs after r76060 is applied.

As described above the events from the thread (progress event and finished event) are unexpectedly dispatched when the progress dialogue is updated (the dialogue internally calls wxYield).
If this would not happen everything would be okay.

Since the events are unexpectedly dispatched by the progress update of the dialogue the following happened in my case:

Thread (a), Thread posts update event
MainThread (i), dialogue.update is called
Thread (b), Thread posts update event
MainThread (ii), dialog calls wxYield
MainThread (iii), dialog.update is called again
Thread (c), Thread posts update event
Thread (d), Thread posts finished event
Thread (e), Thread finished
Nothing happens after the thread is finished which is strange.

Now I would expect that the dialog.update method either returns (and is called again due to the remaining events in the event handler queue) or dispatches the update event (c) and the finished event (d). But nothing happens at all.
My opinion is that the thread is closed and in the same time the internal code of wxYield tries to access the thread's posted events (PeekMessage) which results in undefined behaviour.

This leaves my program in an unusable state because I need the thread finished event to unblock the GUI.

Last edited 4 months ago by hagen (previous) (diff)
Note: See TracTickets for help on using tickets.