fix(lifecycle): recreate the ROM scanner across runtime rebuilds instead of reusing it against a freed bus - #90
Conversation
…st the new bus
ROMScanner, ROMReader and ROMScanSession hold a bare IFireWireBus& fixed at
construction, pointing into the controller-owned FireWireBusImpl.
ServiceContext::Reset destroyed the controller (and with it the bus) but never
dropped deps.romScanner, so EnsureRomScanner reused the stale scanner after
every internal runtime rebuild ("Reusing existing ROMScanner instance") and the
next scan dereferenced freed memory in ROMReader's ReadQuad path (fault offset
0x10 = FireWireBusImpl::async_). Observed as a dext SIGSEGV on real hardware
when scanner power-on triggered a bus reset during a runtime rebuild
(2026-07-29); deterministic on every scan after the first rebuild.
Detach the controller's shared_ptr copy first so ~ROMScanner runs while the bus
is still alive. The destructor only marks the session aborted and queues
weak_ptr cleanup — no bus IO — and every queued FSM entry point checks
aborted_ before touching the bus. EnsureRomScanner then takes its existing
rebuild branch, binding the fresh scanner to the new controller's Bus() and the
new scheduler queue (which was equally stale on reuse).
Pre-existing main bug, surfaced during HW validation of the login-target work;
same class as the IRMClient teardown fix, but the scanner was never rebuilt at
all.
…ds against the new bus" Moved to its own PR (mrmidi#90) — HBA and Config ROM work kept separate per review.
Homebrew's xcodegen moved to 2.46.0, which emits the ASFW/ASFWDriver group order differently than the committed project. CI regenerates and diffs the pbxproj, so every branch fails until the committed file matches the new generator output. main needs the same regeneration on its next change.
|
Hi. Thanks for contributing and splitting HBA/ConfigRom parts to separate PRs. I would still ask you to rebase to dev branch: It conflicts with current dev in DriverContext.cpp because teardown ordering has changed. Place the scanner detach/reset immediately before controller.reset(), preserving the newer audio and lifecycle teardown ordering. |
Sure thing! |
I'm in doubt. See Discord, but probably worth merging it into main — I'll just cherry-pick it for dev. Just because it's fixing existing problem and nobody cares about other branches than main :) |
|
Ok, merging it since we've agreed on Discord. |
2 participants
What
ServiceContext::Resetdestroys the controller (and with it the controller-ownedFireWireBusImpl), but never droppeddeps.romScanner.ROMScanner,ROMReaderandROMScanSessionhold a bareAsync::IFireWireBus&fixed at construction, soEnsureRomScannerreused the stale scanner after every internal runtime rebuild ("Reusing existing ROMScanner instance") and the next scan dereferenced freed memory in theReadQuadpath — fault offset 0x10 =FireWireBusImpl::async_. Deterministic on every scan after the first rebuild.Observed on real hardware (2026-07-29, during HW validation of #61): dext SIGSEGV when scanner power-on triggered a bus reset during a runtime rebuild. Crash report available on request.
Fix
In
Reset, detach the controller's attachedshared_ptrcopy and dropdeps.romScannerbeforecontroller.reset(), so~ROMScannerruns while the bus is still alive.EnsureRomScanner(re-entered fromStartRuntimeon wake/recovery) then takes its existing rebuild branch, binding the fresh scanner to the new controller'sBus()and the new scheduler queue (equally stale on reuse before this fix).Teardown safety:
~ROMScanner→ROMScanSession::Abort()only sets theaborted_flag and queues weak_ptr cleanup — no bus IO — and every queued FSM entry point checksaborted_before touching the bus.Scope
This implements only the safety floor: a scanner must not retain a borrowed bus reference after that bus is destroyed. It does not touch the
ConfigROMStorecache or device identity, and does not change what gets rescanned. Fingerprint-validated rediscovery (preserve cached ROM identity across a reset, revalidate a BIB/root-directory fingerprint before resuming, full rescan only on change — the Apple/Linux model) is deliberately left as the follow-up design it belongs to; nothing here precludes it.Split out of #61 per review feedback (HBA and Config ROM work kept separate). Same bug class as the
IRMClientteardown fix — but the scanner was never rebuilt at all.Testing
./build.sh --scsibuild green✅ ROMScanner createdon the post-wake rebuild (never "Reusing existing ROMScanner instance"), wake-verify alive, and no dext crash. On main this exact sequence SIGSEGVs deterministically.