Opened 7 months ago

Closed 5 months ago

#16222 closed defect (fixed)

crash in wxPropgrid when adding properties during PropertyChange

Reported by: mycae-wx Owned by: AW
Priority: low Milestone:
Component: wxPropertyGrid Version: dev-latest
Keywords: clear properties Cc:
Blocked By: Blocking:
Patch: yes

Description

Dear Wx-developers,

I'm experiencing crashes when attempting to edit a property grid's contents (via ->Clear() and rebuild) during a property change. I am doing this to have "dynamically" visible properties, eg certain properties are visible or not visible depending upon what the user has entered in for values for other properties. Eg, if property "A" has value "1", then show property "C" and its value, otherwise show B and D.

A minimal version of this, demonstrating the crash is below. I have tried Freezing the control, but this simply defers the crash.

I am running under debian testing, using libwxgtk3.0.0-2.

$ valgrind ./main
==19408== Memcheck, a memory error detector
==19408== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==19408== Using Valgrind-3.9.0 and LibVEX; rerun with -h for copyright info
==19408== Command: ./main
==19408== 
==19408== Invalid read of size 8
==19408==    at 0x4EA9550: wxPGProperty::GetY2(int) const (in /usr/lib/x86_64-linux-gnu/libwx_gtk2u_propgrid-3.0.so.0.0.0)
==19408==    by 0x4EB5477: wxPropertyGrid::GetEditorWidgetRect(wxPGProperty*, int) const (in /usr/lib/x86_64-linux-gnu/libwx_gtk2u_propgrid-3.0.so.0.0.0)
==19408==    by 0x4E9951B: wxPropertyGrid::CorrectEditorWidgetPosY() (in /usr/lib/x86_64-linux-gnu/libwx_gtk2u_propgrid-3.0.so.0.0.0)
==19408==    by 0x4EB6007: wxPropertyGrid::RecalculateVirtualSize(int) (in /usr/lib/x86_64-linux-gnu/libwx_gtk2u_propgrid-3.0.so.0.0.0)
==19408==    by 0x4EB6105: wxPropertyGrid::PrepareAfterItemsAdded() (in /usr/lib/x86_64-linux-gnu/libwx_gtk2u_propgrid-3.0.so.0.0.0)
==19408==    by 0x4EB6550: wxPropertyGrid::Refresh(bool, wxRect const*) (in /usr/lib/x86_64-linux-gnu/libwx_gtk2u_propgrid-3.0.so.0.0.0)
==19408==    by 0x4EC7444: wxPropertyGridInterface::Append(wxPGProperty*) (in /usr/lib/x86_64-linux-gnu/libwx_gtk2u_propgrid-3.0.so.0.0.0)
==19408==    by 0x409DAC: SomeFrame::OnGridPropertyChange(wxPropertyGridEvent&) (in /home/pcuser/code/tmp/main)
==19408==    by 0x518C96D: wxAppConsoleBase::CallEventHandler(wxEvtHandler*, wxEventFunctor&, wxEvent&) const (in /usr/lib/x86_64-linux-gnu/libwx_baseu-3.0.so.0.0.0)

Attachments (2)

main.cpp download (1.5 KB) - added by mycae-wx 7 months ago.
Minimal crash example
Allow-clearing-property-grid-inside-event-handlers.patch download (3.2 KB) - added by awi 6 months ago.
Allow clearing whole property grid from within event handler.

Download all attachments as: .zip

Change History (7)

Changed 7 months ago by mycae-wx

Minimal crash example

comment:1 Changed 7 months ago by mycae-wx

For anyone who might follow this up - there is a workaround. You can make the modifications in a side object, then swap them before the event is complete. Do not delete the original grid, except on an idle event. The sizer must be re-created, and then a layout must be run (eg on idle).

comment:2 Changed 6 months ago by awi

  • Keywords clear properties added
  • Patch set
  • Version changed from 3.0.0 to dev-latest

The issue can be observed because at the moment wxPG is not ready to clear its whole content from within event handlers. Calling wxPG::Clear() simply resets all internal data including also the data which are vital to handle the event properly and event handling ends with crash.
In order to allow clearing whole wxPG content from within event handler there is necessary to implement technique of defferred deletion already used to delete individual properties (which is allowed also inside the event handler).
Requests for deletion are recorded (in the list) and deletion itself is done later on when property grid is in the idle state. Patch applying this technique is attached (attachment:ticket:16222:Allow-clearing-property-grid-inside-event-handlers.patch).

Patch to minimal sample (based on the attached code) reproducing the issue is here:

  • samples/minimal/minimal.cpp

    a b  
    3030    #include "wx/wx.h" 
    3131#endif 
    3232 
     33#include <wx/propgrid/propgrid.h> 
     34#pragma comment(lib, "wxmsw31ud_adv.lib") 
     35#pragma comment(lib, "wxmsw31ud_propgrid.lib") 
     36 
    3337// ---------------------------------------------------------------------------- 
    3438// resources 
    3539// ---------------------------------------------------------------------------- 
    public: 
    6872    void OnQuit(wxCommandEvent& event); 
    6973    void OnAbout(wxCommandEvent& event); 
    7074 
     75    void OnGridPropertyChange(wxPropertyGridEvent &event); 
    7176private: 
     77    wxPropertyGrid* m_grid; 
    7278    // any class wishing to process wxWidgets events must use this macro 
    7379    wxDECLARE_EVENT_TABLE(); 
    7480}; 
    private: 
    7682// ---------------------------------------------------------------------------- 
    7783// constants 
    7884// ---------------------------------------------------------------------------- 
     85enum 
     86{ 
     87    ID_PROPGRID=wxID_ANY+1, 
     88}; 
    7989 
    8090// IDs for the controls and the menu commands 
    8191enum 
    enum 
    99109wxBEGIN_EVENT_TABLE(MyFrame, wxFrame) 
    100110    EVT_MENU(Minimal_Quit,  MyFrame::OnQuit) 
    101111    EVT_MENU(Minimal_About, MyFrame::OnAbout) 
     112    EVT_PG_CHANGING(ID_PROPGRID, MyFrame::OnGridPropertyChange) 
    102113wxEND_EVENT_TABLE() 
    103114 
    104115// Create a new application object: this macro will allow wxWidgets to create 
    MyFrame::MyFrame(const wxString& title) 
    172183    CreateStatusBar(2); 
    173184    SetStatusText("Welcome to wxWidgets!"); 
    174185#endif // wxUSE_STATUSBAR 
     186    m_grid = new wxPropertyGrid(this, ID_PROPGRID); 
     187    m_grid->Append( new wxPropertyCategory("Category 1")); 
     188        m_grid->Append(new wxStringProperty("Name","Name","some data")); 
     189 
     190    wxBoxSizer* sizer = new wxBoxSizer(wxVERTICAL); 
     191    sizer->Add(m_grid, 1, wxEXPAND, 0); 
     192        SetSizer(sizer); 
     193        sizer->Fit(this); 
     194        Layout(); 
    175195} 
    176196 
    177197 
    void MyFrame::OnAbout(wxCommandEvent& WXUNUSED(event)) 
    198218                 wxOK | wxICON_INFORMATION, 
    199219                 this); 
    200220} 
     221 
     222void MyFrame::OnGridPropertyChange(wxPropertyGridEvent& WXUNUSED(event)) 
     223{ 
     224        m_grid->Clear(); 
     225    m_grid->Append( new wxPropertyCategory("Category 2")); 
     226    m_grid->Append(new wxStringProperty("New Name","New Name","some data")); 
     227} 

Changed 6 months ago by awi

Allow clearing whole property grid from within event handler.

comment:3 Changed 5 months ago by AW

In 76888:

Prevent duplicating wxPG property items on the list of items for deferred deletion/removal.

It is necessary to prevent duplicating items on the list of items to be deleted/removed later on (in wxPropertyGrid::OnIdle) to avoid crashes when it would be attempted to delete/remove already deleted/removed item.

See #16222.

comment:4 Changed 5 months ago by AW

In 76889:

Adjust list of items for deferred deletion/removal if wxPG property item is actually deleted/removed.

When property is actually deleted/removed it must be also removed from the respective list of items for deferred deletion/removal in order to avoid crashes when it would be attempted to delete/remove it again at next wxPG idle state.
Because lists of items for deferred operations can be updated at every actual deletion/removal it is necessary to rearrange iteration through these lists in wxPG::OnIdle.

See #16222.

comment:5 Changed 5 months ago by AW

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

In 76890:

Allow clearing wxPG from within wxPG event handlers.

If wxPG::Clear is called from within event handler then it is not possible to delete all property items directly because some vital internal wxPG data are still in use. In this case it is necessary to put all items on the list for deferred deletion in wxPG idle state.

Closes #16222.

Note: See TracTickets for help on using tickets.