#14727 closed defect (fixed)

Application using wxListCtrl is getting a "wxYield() called recursively" error

Reported by: jdagresta Owned by:
Priority: normal Milestone: 2.9.5
Component: wxGTK Version: 2.9.4
Keywords: wxListCtrl wxYield Linux Cc: jdagresta@…
Blocked By: Blocking:
Patch: yes

Description

Our application is like Windows Explorer in that it has a wxTreeCtrl side-by-side with a wxListCtrl.

One of our customers, on the Linux platform, has the problem that sometimes when they select an item in the Tree control (which initiates and update of the List control) they get a pop-up error window with a "wxYield() called recursively" message.

We have been unable to reproduce the problem in-house, but the customer can easily reproduce the problem (and have sent us screen snaps of the error pop-up).

I see in the wxWidgets code the warnings about using wxYield() and the potential for this kind of problem and the alternate "safer" routines that have been provided.

I see that all of the calls to wxYield() in the Tree control code have been replaced by wxYieldIfNeeded() calls.

Then I noticed that src/generic/listctrl.cpp contains a call to wxSafeYield(), but without specifying 'true' for the onlyIfNeeded argument (which defaults to 'false').

I tried a test where I changed the wxSafeYield() to wxSafeYield(NULL, true), which is the equivalent of changing wxYield() to wxYieldIfNeeded(), and sent our application with that change to the customer and they've verified that it fixes the wxYield() called recursively problem.

I propose that this change be made to the wxWidgets code; proposed patch is attached.

Attachments (1)

listctrl.patch download (891 bytes) - added by jdagresta 19 months ago.
Proposed patch to src/generic/listctrl.cpp

Download all attachments as: .zip

Change History (4)

Changed 19 months ago by jdagresta

Proposed patch to src/generic/listctrl.cpp

comment:1 Changed 19 months ago by vadz

  • Milestone set to 2.9.5

I'm not against this change per se and can apply it but I think something is seriously wrong if EditLabel() ends up being called from inside wxYield(). This really would merit being debugged, although I realize it can be difficult if you can't reproduce the problem yourself.

Also, looking at the code, I wonder if we need yielding here at all. Might calling Update() be enough instead? Could you please check if doing this works for your application?

comment:2 Changed 18 months ago by jdagresta

I created a version of our application using Update(), rather than the change to use wxSafeYield(NULL, true), and sent it off to be tested by our customer that was getting the "wxYield() called recursively" error.

The customer has done some limited testing and reported back that the application was working fine in the areas that were causing the wxYield() error. They plan to install the app on their test server for further testing.

Given what the comments say in listctrl.cpp around where the wxSafeYield() call is located, I'll leave it up to you to decide whether the wxSafeYield() should just be changed to the "even safer" version with the "NULL, true" parameters (as I initially proposed) or whether you want to completely change that to just use Update().

Whatever you decide I will "permanently" make the same changes to our version of wxWidgets and our application (for all our customers), once the changes are applied.

comment:3 Changed 18 months ago by VZ

  • Resolution set to fixed
  • Status changed from new to closed

(In [72754]) Don't call wxSafeYield() from wxGenericListCtrl::EditLabel().

This could result in wxYield() reentrancy and while it could be avoided by
using wxSafeYield(NULL, true /* only if needed */) it seems that we don't
actually need to yield here at all and a simple Update() should be enough.

Closes #14727.

Note: See TracTickets for help on using tickets.