[wx-dev] #9712: ClearAttrCache bug causes wxGrid::Redimension crashes when called while a GridCellEditor is open

[wxTrac]

Ticket URL: <http://trac.wxwidgets.org/ticket/9712>

#9712: ClearAttrCache bug causes wxGrid::Redimension crashes when called while a
GridCellEditor is open
-----------------------------------------------------------+----------------
 Reporter:  hajokirchhoff                                  |       Owner:         
     Type:  defect                                         |      Status:  new    
 Priority:  high                                           |   Milestone:         
Component:  wxGrid                                         |     Version:  2.9-svn
 Keywords:  crash memory corruption cell attribute editor  |   Blockedby:         
    Patch:  0                                              |    Blocking:         
-----------------------------------------------------------+----------------
 wx-svn four weeks ago, wxMSW, Visual Studio 2005

 If wxGrid::Redimension is called while a GridCellEditor is open and has
 the focus, the application tries to access freed memory on the heap and
 crashes.

 Redimension()
 {
    ClearAttrCache();
    HideCellEditControl();
 ...

 ClearAttrCache calls wxSafeDecRef for the cached cell attribute.
 This will call 'delete this' for the attribute.
 This will call wxSafeDecRef for the attributes grid cell editor.
 This will call 'delete this' for the grid cell editor.
 This will call ~wxWindow()
 This causes a wxEVT_KILL_FOCUS event when the grid cell editor looses the
 focus.
 wxGridWindow::OnFocus processes this event. Its children (callees) access
 the current cell attribute.

 m_attrCache still appears to be valid, even though delete m_attrCache.attr
 has already been called. As a result, LookupAttr() will return a pointer
 to an attribute that has already been deleted.


 The bug is in ClearAttrCache()

 void wxGrid::ClearAttrCache()
 {
     if ( m_attrCache.row != -1 )
     {
         wxSafeDecRef(m_attrCache.attr);
         m_attrCache.attr = NULL;
         m_attrCache.row = -1;
     }
 }

 wxSafeDecRef(...) deletes the attribute, but m_attrCache.row is still
 valid.
 Deleting the cached attribute can trigger events that query the cached
 attribute.
 But now m_attrCache.attr is invalid while m_attrCache.row is still valid
 (!=-1).

 To correct this, the m_attrCache should be marked as invalid before the
 attribute is deleted. Also, m_attrCache.attr needs to be set to NULL
 before the attribute is deleted, because deleting the attribute might set
 m_attrCache.attr to a different attribute.

 if (m_attrCache.row!=-1)
 {
   // mark cache as invalid
   wxGridCellAttr *oldAttr = m_attrCache.attr;
   m_attrCache.row = -1;
   m_attrCache.attr = NULL;
   wxSafeDecRef(oldAttr);
 }

 I'll prepare a proper patch.


--
Ticket URL: <http://trac.wxwidgets.org/ticket/9712>