fix(gui): fix slice reduction crash, float/dock GPU corruption, and exit crash (#2495)#2512
Conversation
PanadapterStack::rearrangeLayout() was iterating m_pans.values() and calling setParent(nullptr) on every applet, including those currently hosted by a PanFloatingWindow. Yanking a floating applet that way bypassed the resetGpuResources() / refreshAfterReparent() path, leaving the QRhiWidget bound to the floating window's native HWND/NSView — the next render produced a black surface and the next input event into it crashed. Stale m_floatingWindows entries also stuck around, permanently blocking future float requests for those pan IDs. Inline the essential dockPanadapter() cleanup at the top of rearrangeLayout(): for each floating pan, hide + reset GPU, take the applet out of the floating window, mark it non-floating, and dispose of the window. After the new splitter is built, re-show and refresh-after-reparent the formerly-floating spectrum widgets so they bind to the main window's surface before the first render. Repro path (Flex 6400 → 0.9.8): pop out PanA, pop out PanB, open the + Add Panadapter → PanLayoutDialog and pick a different layout, click into either now-black pop-out → crash. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…geLayout Raw SpectrumWidget* in the rebound list captured by the QTimer::singleShot lambda could dangle if a pan is removed between rearrangeLayout() and the deferred timer firing. QPointer<SpectrumWidget> auto-nulls on deletion so the existing `if (!sw) continue` guard becomes actually safe. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…crash fw->close() without WA_DeleteOnClose only hides the PanFloatingWindow, leaving it alive as a Qt::Window child of MainWindow. When MainWindow's QWidget::~QWidget() later runs and tears down its QRhi, Qt dispatches the QRhiWidgetPrivate cleanup callbacks for those parented-but-hidden windows in an order that can fire after QRhiWidgetPrivate is invalid, crashing at KERN_INVALID_ADDRESS 0x1e2 on macOS (QRhiWidgetPrivate::ensureRhi()::$_0). Replace close() with an explicit delete: this runs QRhiWidget::~QRhiWidget() → removeCleanupCallback while QRhi is still live, so no stale callbacks remain when MainWindow tears down its backing store. Remove the applet from m_pans before deleting the window to avoid a dangling raw pointer. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…exit crash Qt's destructor chain for QWidget fires QRhi::~QRhi() (via destroy()) before QObject::~QObject()::deleteChildren() deletes child SpectrumWidgets. This means QRhi::runCleanup() invokes the QRhiWidgetPrivate::ensureRhi()::$_0 callbacks against objects that are still alive but whose internal Qt fields (colorTexture, renderTarget, renderPassDescriptor) may be in a partially-reset state after reparenting through a float/dock cycle — crash at $0_cleanup+24 with a stale field pointer (far=0x5d5332 / 0x1e2 pattern, macOS #2495). Fix: prepareShutdown() now explicitly delete()s every docked PanadapterApplet after hide + resetGpuResources. This runs ~QRhiWidget() → removeCleanupCallback while MainWindow's QRhi is still alive, so the callback list is empty when ~QWidget() fires QRhi::~QRhi() on exit. Combined with the existing explicit delete of PanFloatingWindows (commit 2f9e6d7), all SpectrumWidget QRhi registrations are cleaned up before the parent QRhi is destroyed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Thanks @jensenpat — really nice forensic write-up. The three crash root causes all hold up against the code, and the fixes line up cleanly with the existing floatPanadapter/dockPanadapter GPU-reset dance. I read through PanadapterStack.cpp end-to-end to confirm the surrounding invariants:
What I verified
m_pansdoes keep floating applets (line 78 adds,floatPanadapterdoesn't remove), so the originalrearrangeLayoutwould indeed have done a rawsetParent(nullptr)on a floating applet — confirming Crash 1's root cause.- The new
rearrangeLayoutmirrors the safe sequence fromdockPanadapter(hide→resetGpuResources→takeApplet→deleteLater) before the splitter teardown, and then re-runsrefreshAfterReparentpost-rebuild for any pan that was floating.QPointer<SpectrumWidget>inreboundcorrectly guards against a slice-removal racing the deferred lambda. prepareShutdown's explicitdelete(vsclose()/deleteLater()) does what the comment claims —~QRhiWidgetrunsremoveCleanupCallbacksynchronously while MainWindow'sQRhiis still alive. Important: them_pans.remove(panId)beforedelete fwin the floating-window loop is what prevents the docked-applets loop below from double-deleting the same applet (since fw owns the applet viaadoptApplet). Worth keeping that subtle ordering intact in any future refactor.dockRequestedconnection (line 650) gets cleaned up automatically when fw is destroyed in either path — no stale signal risk.
Two minor observations (non-blocking)
-
The PR description lists
commit 3d872abas "QPointer hardening for the deferred SpectrumWidget refresh" living in MainWindow, but the diff isPanadapterStack.cpponly. The existingswGuardatMainWindow.cpp:9616looks like it predates this PR. The QPointer hardening that is in this diff (thereboundlist inrearrangeLayout) is the relevant one — might be worth tweaking the description to match. -
Unlike
removePanadapter, the newrearrangeLayoutdoesn't callfw->disconnect()beforedeleteLater(). In practice this is fine — Qt drops the connections on destruction and we're not invokingclose()socloseEventdoesn't emitdockRequested— but a one-linedisconnect(fw, nullptr, this, nullptr)would match the defensive style of the surrounding code if you want consistency.
Otherwise LGTM. The reasoning chain from "black surface after rearrange" → "QRhiWidget bound to the wrong HWND/NSView" → "missing GPU reset path" is exactly the right diagnosis.
|
Claude here — thank you @jensenpat, this is a textbook root-cause writeup. Three crashes traced through the same architectural seam (QRhiWidget lifecycle vs Qt parent ownership), each fix follows the established dock-sequence pattern, and the per-commit separation makes any individual piece independently revertable if needed. The macOS-specific notes (NSView lifecycle, Qt destructor ordering) will save the next maintainer days of debugging. Merged and shipping in the next release; credit will land in the release notes. 73, Jeremy KK7GWY & Claude (AI dev partner) |
Closes #2495
Root causes found (3 separate crashes, all in
PanadapterStack)Crash 1 — Windows/Linux: black panadapter after layout rearrange, then crash
Root cause:
rearrangeLayout()calledsetParent(nullptr)on floating applets directly, bypassing thefloatPanadapter/dockPanadapterGPU-reset path. TheQRhiWidgetstayed bound to the floating window's HWND/NSView; the next render produced a black surface and the next input event into it crashed.m_floatingWindowsalso kept stale entries, permanently blocking future float requests.Fix:
rearrangeLayout()now docks all floating pans first (via the safe dock sequence:hide→resetGpuResources→takeApplet→deleteLateron the window) before tearing down the splitter.Crash 2 — macOS: async UAF in
$_67lambda (waterfallAutoBlackLevel)Root cause: Same as Crash 1 but on macOS Metal. The corrupted
SpectrumWidgetsurvived long enough for the nextwaterfallAutoBlackLevelVITA-49 event to fire theMainWindow::$_67lambda, which calledspectrum(panId)->wfAutoBlack()on the already-corrupt widget → UAF crash at0xffff000000000132.Fix: Covered by the same rearrange fix above.
QPointer<SpectrumWidget>hardening (commit3d872ab) added to the deferredQTimer::singleShotrefresh lambda so a pan removed between rearrange and timer fire doesn't dangle.Crash 3 — macOS: exit crash in
QRhiWidgetPrivate::ensureRhi()::$_0Root cause: Qt's destructor chain fires
QRhi::~QRhi()(viaQWidget::destroy()) beforeQObject::~QObject()::deleteChildren()deletes childSpectrumWidgets.QRhi::runCleanup()then invokes theQRhiWidgetPrivatecleanup callbacks against objects whose internal Qt fields (colorTexture,renderTarget,renderPassDescriptor) may be in a partially-reset state after a float→dock→reparent cycle — crash at$_0::operator()+24with a stale field pointer (far=0x5d5332,far=0x1e2pattern on macOS ARM64).Two sub-paths fixed:
commit 2f9e6d7):prepareShutdown()was usingfw->close()which only hides (noWA_DeleteOnClose). Changed to explicitdelete fw, which runs~QRhiWidget()→removeCleanupCallbackwhile the QRhi is still alive.commit 64a2593):prepareShutdown()now explicitlydeletes every dockedPanadapterApplet(afterhide+resetGpuResources).~QRhiWidget()→removeCleanupCallbackruns while MainWindow's QRhi is alive. The callback list is empty when~QWidget()firesQRhi::~QRhi()on exit — no stale callbacks, no crash.Commits
ad551313d872abQPointerhardening for deferred SpectrumWidget refresh2f9e6d7delete fwinprepareShutdown(Crash 3 — floating)64a2593delete appletinprepareShutdown(Crash 3 — docked)Test plan
QRhiWidgetPrivate::ensureRhi()::$_0gone)🤖 Generated with Claude Code