FIX: Guard m_side_window against null/stale pointer in ObjectGridTable (fixes Flatpak crash on 'View all object settings')#10303
Conversation
Crash reproducible on Linux Flatpak (GNOME Platform 48) when clicking 'View all object settings'. Reported in issues bambulab#9423, bambulab#10015, bambulab#10178. Root cause: ObjectGridTable::OnSelectCell(), OnCellValueChanged(), and OnCellLeftClick() all call m_panel->m_side_window->Freeze() without verifying that m_side_window is still valid. On Flatpak with GNOME Platform 48, the newer wxWidgets ABI triggers an invalid read at address 0x1cc (Freeze() dereferencing a stale vtable pointer), causing a SIGSEGV. Valgrind output from issue bambulab#9423: Invalid read of size 4 in wxWindowBase::Freeze() called from ObjectGridTable::OnSelectCell() Changes: 1. OnSelectCell(): early return if m_side_window is null 2. OnCellLeftClick(): wrap Freeze/ValueChanged/Thaw in null check 3. OnCellValueChanged(): wrap Freeze/ValueChanged/Thaw in null check 4. ~ObjectTablePanel(): set m_side_window = nullptr after Clear(true) so that any pending wxGrid events fired during teardown hit the guard rather than a freed pointer Note: this addresses the symptom (crash) safely. The root cause is an ABI mismatch between the wxWidgets version bundled in the AppImage and the one provided by org.gnome.Platform/48 in the Flatpak runtime. Co-authored-by: GitHub Copilot <copilot@github.com> AI-generated: This fix was developed with the assistance of GitHub Copilot (Claude Sonnet 4.6). The logic and diagnosis were reviewed and validated by the contributor.
ABI shouldn't factor into it at all, because we're recompiling from source. So either there's a warning during the build which we're ignoring and we shouldn't, or it's a bug that just happens not to crash on the AppImage version because, for example, FORTIFY_SOURCE isn't set during its build. If the problem is an ABI/API problem, then we should be able to find the exact upstream commit that started causing crashes. |
|
Thanks @hadess — that makes sense. I've updated the PR description to remove the ABI mismatch explanation and mark the root cause as unconfirmed. The intent of this PR is narrower: add a defensive null-guard around I don't have a Flatpak build environment, so I can't reproduce or further diagnose from my side. If you have a direction you'd like me to investigate (build warnings, compiler flags, a specific wxWidgets upstream commit), I'm happy to look into it. |
Crash reproduced locallyEnvironment:
Steps to reproduce:
Kernel crash log ( The crash address The offset |
…e window Previously m_side_window was nulled after m_top_sizer->Clear(true), which deletes the widget. Pending wxGrid events (e.g. EVT_GRID_SELECT_CELL) could fire between the delete and the null assignment, bypassing the null-guards in OnSelectCell/OnCellLeftClick/OnCellValueChanged and causing use-after-free. Move the null assignment before Clear(true) so any pending event hits the guard before the widget is destroyed.
Summary
Fixes a crash when clicking View all object settings on Linux Flatpak (GNOME Platform 48). Reported in #9423, #10015, #10178.
Crash location
Valgrind output from issue #9423 (credit: @hadess):
OnSelectCell(),OnCellValueChanged(), andOnCellLeftClick()all callm_panel->m_side_window->Freeze()without verifying thatm_side_windowis still valid, which is the crash path identified in the Valgrind trace.Root cause
The crash is a use-after-free during teardown caused by a race between
ObjectTablePaneldestruction and pending wxGrid events.The original destructor:
m_side_windowis a child widget owned bym_top_sizer.Clear(true)deletes it. wxWidgets can process pending events (e.g.EVT_GRID_SELECT_CELL) during widget destruction — at that pointm_side_windowis already deleted but the pointer has not been nulled yet, so the null-guard inOnSelectCell()does not help. The result is an invalid read at0x1cc(m_freezeCountinwxWindowBase), exactly as seen in the Valgrind trace.The fix is to null the pointer before
Clear(true):Changes
~ObjectTablePanel(): nullm_side_windowbeforem_top_sizer->Clear(true)to close the race windowOnSelectCell(): early return ifm_side_windowis nullOnCellLeftClick(): wrap Freeze/ValueChanged/Thaw in null checkOnCellValueChanged(): wrap Freeze/ValueChanged/Thaw in null checkThe null-guards in the event handlers are still necessary as a defence in depth — the destructor fix closes the primary race, but the guards protect against any other path where
m_side_windowmight be null or stale.Testing
Not runtime-tested by the contributor (no Flatpak build environment available). The fix is based on static code analysis of the destructor and the crash path from the Valgrind trace. The crash has been reproduced locally on BambuStudio 2.5.0.66 Flatpak + GNOME Platform 48 (see comments below). If a maintainer or contributor can confirm the fix resolves the crash, that would be very helpful.
AI Disclosure: This fix was developed with the assistance of GitHub Copilot (Claude Sonnet 4.6). The diagnosis, code analysis, and validation were performed by the contributor. The AI assisted with code generation and diff review.