Opened 4 years ago

Last modified 4 years ago

#13053 confirmed defect

Application hung (infinite loop) if the same class-name is used twice for different classes with event tables

Reported by: gal_wxwidgets Owned by:
Priority: low Milestone:
Component: base Version: stable-latest
Keywords: Cc: gal_wxwidgets@…
Blocked By: Blocking:
Patch: yes

Description

See also #13051 - for older 2.8.x branch

Notes and references

The bug + patch is also available with Ubuntu bug https://launchpad.net/bugs/735462

What you expected to happen

The wxwidgets should check for duplicate insertion into the event table and flag a duplicate insert of the same class as an error

What happened instead

The second insert (of the same table) did succeed and create a cycle within the linked list.
On initialization, the initialization of the linked list entered an infinite loop

Description of patch

Add a check for the error case of "duplicate insert into the linked-list of the event tables"

Details

If a duplicate insert is detected, then issue an error and avoid the re-insert. It is then, the responsibility of the developer of the program, to fix the error within his code and to avoid using the same class name twice.

Reproducing the error case that this patch detect

  1. Create a class (within your program) that has the same class-name as a class that already exist within the wxwidgets code. Both classes should have event tables.
  2. Compile this code into your executable
  3. Link with a shared version of the wxwidgets libraries
  4. Start the application:
    1. The executable is initiated (before calling main()) - and the event table is added to the link list for the first time
    2. The shared libraries of the wxwidgets are initiated
      1. The event table from the wxwidgets shared library is resolved to the symbol from the executable (with the same name) and NOT to the symbol within the shared library
      2. The SAME event table is added for the SECOND time into the linked list and create a cycle
  5. main() is called
    1. wxwidget is initialize - and it get stack with an infinite loop when trying to initiate the linked list of the event table

Creating the patch

  1. Get sources
    svn checkout
    svn checkout http://svn.wxwidgets.org/svn/wx/wxWidgets/trunk wxWidgets
    
  2. Apply the patch at http://trac.wxwidgets.org/browser/wxWidgets/trunk/src/common/event.cpp#L863
  3. get diff
    svn diff wxWidgets > check-for-cycle-dependency.patch
    

Attachments (1)

check-for-cycle-dependency.patch download (1.1 KB) - added by gal_wxwidgets 4 years ago.
check-for-cycle-dependency.patch

Download all attachments as: .zip

Change History (6)

comment:1 Changed 4 years ago by vadz

  • Milestone 2.9.2 deleted
  • Priority changed from normal to low
  • Status changed from new to confirmed

I'm not sure if we want to apply this patch. It does solve the problem but at the cost of making the insertion into the event hash tables linear time instead of constant time which risks to have noticeable effect on the program initialization time. And the problem is, after all, pretty noticeable: it's not like you can ignore it if the program just hangs. And it should be relatively simple to diagnose too as it hangs inside a loop which clearly indicates that the loop is infinite.

I'm also not sure how can something like this arise accidentally nor if avoiding the problem here enough to avoid other potential problems due to the presence of two classes with the same name. Which is, of course, completely illegal to begin with as it breaks the ODR.

To summarize, if we could insert a simple check to make diagnosing/debugging this problem easier I'd be ready to do it but I don't think we want to pay for this very rarely needed check on startup of every wx application.

Any other opinions?

comment:2 Changed 4 years ago by disc

If performance is an issue I wouldn't mind having this in just debug mode (though that mode is getting rare in wx I guess) to start with (note that I have never been affected by this problem). In case end-users can trigger this with an wx IDE or the like then I think the check should (also) be in the respective program.

Changed 4 years ago by gal_wxwidgets

check-for-cycle-dependency.patch

comment:3 Changed 4 years ago by gal_wxwidgets

In case end-users can trigger this with an wx IDE ...

The problem was triggered by a bad GUI-builder tool that had created a bad code :-(

If performance is an issue I wouldn't mind having this in just debug mode ...

Regenerate the patch within #if defined(WXDEBUG) ... #endif

comment:4 Changed 4 years ago by vadz

Notice that __WXDEBUG__ is always defined by default now (in wx 2.9/3.x) so this code would still be enabled by default and IMO still not worth it. More I think about this, more I suspect that having two classes with the same names will result in other problems anyhow and less I see the point of checking for this here...

comment:5 Changed 4 years ago by gal_wxwidgets

vadz> Notice that WXDEBUG is always defined by default now ...
Gal:
So what is the define that should be used instead?
From include/wx/debug.h:
WXDEBUG is defined when wxDEBUG_LEVEL != 0. This is done mostly for

compatibility but it also provides a simpler way to check if asserts and
debug logging is enabled at all.

vadz> more I suspect that having two classes with the same names will result in other problems anyhow and less I see the point of checking for this here...
Gal:
To my grief - I had the miss-opportunity of debugging such a code that was generated by some bugy tool for automatic generating a wxwidget-GUI.
The problem (of two classes with same name) can be detected by the linker only when doing a link with static versions of the wxwidgets libraries.

Note: See TracTickets for help on using tickets.