Fix dangling pointer in propgrid (PR #26385)
Randalphwa
On Windows 11, in wxUiEditor, its propgrid now generates an assert in src\common\dcbufcmn.cpp
'''cpp
void wxBufferedDC::UnMask()
{
wxCHECK_RET( m_dc, wxT("no underlying wxDC?") );
wxASSERT_MSG( m_buffer && m_buffer->IsOk(), wxT("invalid backing store") );
``
m_buffer is invalid because it's been deleted.
The root cause of the bug is in propgrid.cpp at line 594:
if ( m_doubleBuffer ) delete m_doubleBuffer;
This deletes the buffer but does NOT null the pointer. During destruction, Windows sends a WM_PAINT message, which invokes OnPaint(). The code checks m_doubleBuffer != nullptr (it's still non-null — dangling pointer!), creates a wxBufferedPaintDC referencing the freed bitmap, and then the assert fires when UnMask() tries to check m_buffer->IsOk() on freed memory (m_refData = 0xFEEEFEEE).
The first fix is to use wxDELETE instead of delete, which deletes AND nullifies the pointer.
The second fix is in OnPaint() to check for destruction-in-progress before creating the wxBufferedPaintDC:
// Don't paint after destruction has begun if ( !HasInternalFlag(wxPG_FL_INITIALIZED) ) { wxPaintDC dc(this); return; }
What Changed in wxWidgets:
In include/wx/dcbuffer.h, the macro wxALWAYS_NATIVE_DOUBLE_BUFFER changed:
| Version | Value on Windows |
|---|---|
| Old | 1 (unconditional for ALL platforms) |
| New | 0 (Windows-specific: #ifdef WXMSW → 0) |
This single change has a cascading effect: it switches wxAutoBufferedPaintDC from wxPaintDC to wxBufferedPaintDC on Windows, and causes wxPropertyGrid to use its m_doubleBuffer bitmap for software double-buffering — a code path that was dead code on Windows in the old version.
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/26385
Commit Summary
- bb62aba Fix dangling pointer in propgrid
File Changes
(1 file)
- M src/propgrid/propgrid.cpp (10)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/26385.patch
- https://github.com/wxWidgets/wxWidgets/pull/26385.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26385)This looks LLM-generated (?), but seems correct even if so.
However it's not the first time we have problems due to handling WM_PAINT while the window is already half-destroyed, see e.g. 3e32a9a (Don't bother resetting wxStaticBitmap image when destroying it, 2025-06-12) and I think there were other similar cases. So I wonder if it we could check IsBeingDeleted() in wxWindow::HandlePaint() for a more universal fix? We would still need to call SendDestroyEvent() in wxPropertyGrid::~wxPropertyGrid(), but it's a good thing to do anyhow.
What do you think?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Randalphwa
@Randalphwa pushed 1 commit.
- 2dda9ec Switch OnPaint fix to wxWindow::HandlePaint()
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Randalphwa
Randalphwa left a comment (wxWidgets/wxWidgets#26385)Good idea -- proving once again why human code reviewers are still invaluable.
I used a custom debugging agent hooked up to a custom version of lldb-dap designed for agents to call, pointed it to the ASSERT and let it walk up the call stack to find the root cause. It found the issue, made the fix, and I verified that wxUiEditor stopped crashing. I then switched over to my fork, checked to see if propgrid would hit the assert (it didn't) and then manually copied and pasted the fixes from the wxUiEditor wxWidgets copy, and verified that nothing appeared to be broken in the propgrid sample. The summary the agent gave was arguably more detail than needed, but it did seem to make it clear what the issue was and why it was breaking now.
However, that's as far as it went -- and since I'm not at all familiar with the propgrid code, I didn't look any further, figuring (rightly!) that someone who was more familiar with the double-buffering changes would have a better suggestion if there was one.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26385)Hmm, I don't think this is enough, without SendDestroyEvent() there is still a window of time between wxPG dtor and base wxWindow dtor when we could get an unwanted repaint.
But I can add it myself when merging this if you didn't do it for some reason I'm missing.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Randalphwa
@Randalphwa pushed 1 commit.
- 1b07a04 Call SendDestroyEvent90 in dtor, prior to teardown
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26385)Thanks, I'll push it soon but I'll leave out nulling m_doubleBuffer, it shouldn't be necessary any more.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.