Opened 12 months ago

Closed 12 months ago

Last modified 2 weeks ago

#15642 closed defect (fixed)

wxPropertyGrid::OnSysColourChanged leads to access violation

Reported by: hajokirchhoff Owned by: RD
Priority: normal Milestone:
Component: wxPropertyGrid Version: stable-latest
Keywords: Cc:
Blocked By: Blocking:
Patch: yes

Description

A rare occurrence under normal circumstances, but if a WM_SYSCOLOR_CHANGE event is fired during the construction of a wxPropertyGrid, the code will cause an access violation.

Here are the details: We are using a skinning engine that sends a WM_SYSCOLOR_CHANGE event to every window immediately after it is created. wxPropertyGrid::OnSysColorChanged() calls "RegainColours", which in turn tries to access m_categoryDefaultCell. This variable is not initialised until after a call to Init2(), which happens at the end of Create().

Thus, the order of events is this:

  1. wxPropertyGrid::Create() creates the window.
  2. A WM_SYSCOLOR_CHANGE event is received and
  3. OnSysColourChanged() calls RegainColours(), which
  4. tries to store the color in m_categoryDefaultCell
  5. which has not yet been initialised because Init2() has not been called.

My proposed fix simply suppresses the WM_SYSCOLOR_CHANGE event until Init2() has been called. This should work fine, since the colours will be initialised in Init2(). IOW, any WM_SYSCOLOR_CHANGE event before this point has no real effect anyway as the colours will be set correctly.

Attachments (1)

propgrid.cpp.patch download (502 bytes) - added by hajokirchhoff 12 months ago.

Download all attachments as: .zip

Change History (5)

Changed 12 months ago by hajokirchhoff

comment:1 Changed 12 months ago by vadz

Thanks, I'll apply this, but IMHO accesses to m_categoryDefaultCell should also be checked...

comment:2 Changed 12 months ago by VZ

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

(In [75128]) Ignore system colour change events in not yet initialized wxPropertyGrid.

Handling these events before the initialization was over could result in a
crash because m_categoryDefaultCell wasn't yet initialized when the window was
created. Fix this by ignoring these events during initialization.

Closes #15642.

comment:3 Changed 2 weeks ago by RD

  • Owner set to RD

In 77969:

Fix use of log.warn. Fixes #15642

comment:4 Changed 2 weeks ago by ojwb

The previous message was meant for #15462 - I've just commented on and closed that ticket.

Note: See TracTickets for help on using tickets.