Fix two macOS crashes: lld exception unwinding in executables, and payload-less segments aborting the app - #1225
Open
JamesDarby345 wants to merge 3 commits into
Conversation
ld64.lld mishandles C++ exception unwind info for our large ThinLTO binaries: a `throw` that should be caught by an enclosing handler instead unwinds past every handler into std::terminate. The workaround for this already exists but covers SHARED_LIBRARY targets only, because the failure was first seen for cross-dylib throws. With Homebrew clang/lld 22.1.8 the same corruption appears *within* the VC3D executable. Opening any volume package makes CState::setVpkg emit before a volume is loaded, so CWindow::updateAtlasFiberDocks calls resolveLasagnaForVolume with an id that is not in loadedVolumes_ and it throws — a normal, expected condition that the `catch (...)` a few frames up exists to swallow. Instead the process aborts: libc++abi: terminating due to uncaught exception of type std::runtime_error: Active volume is not loaded for Lasagna shape pairing The crash report shows __cxa_throw -> failed_throw -> std::terminate with the enclosing handler still on the stack, i.e. unwinding found no handler that is plainly there. Relinking the same objects and the same dylibs with Apple's ld — nothing else changed — turns that back into the intended handled path, and the same swap fixes an unrelated uncaught json parse_error when opening a malformed project. So executables need the workaround too. Costs some link time on macOS in exchange for exceptions that work. CI did not catch this because it pins an older dependency set whose lld still linked executables correctly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A segment directory carrying meta.json but no x/y/z.tif — a partial Open
Data download, an interrupted save, a segment still being written — passes
Segmentation::canLoadSurface() today, because that only checks
format == "tifxyz". loadSurface() then hands back a QuadSurface whose
payload is read lazily by ensureLoaded() on first geometry access, long
after loadSurface() returned and far from its try/catch.
That first access typically happens inside a Qt slot. Reloading surfaces
over a partially downloaded sample aborts the session:
SurfacePanelController::loadSurfacesIncremental
-> CState::setSurface -> [signal] -> onSurfaceChanged
-> updateFocusMarker -> volumeToScene -> QuadSurface::pointTo
-> ensureLoaded -> load_quad_from_tifxyz -> throw
libc++abi: terminating due to uncaught exception of type
std::runtime_error: Failed to open TIFF: .../x.tif
Nothing in that chain catches. The same error is already survivable one
frame over — ViewerManager's SurfacePatchIndex task wraps its call and logs
"single-surface task failed" — which is why one segment warns and the next
one kills the app.
Two changes:
- canLoadSurface() also requires the payload to exist, so an incomplete
directory is simply not loadable. The failure stays at registration time,
where loadSurface() already returns nullptr and callers handle it.
- onSurfaceChanged() becomes a thin guard around onSurfaceChangedImpl(),
so a payload that goes missing or unreadable *after* registration (file
deleted mid-session, corrupt TIFF, unmounted share) logs and skips the
surface instead of terminating.
The guard has to sit inside the slot rather than above it. Exceptions do
not propagate through Qt's dispatch frames: with a throwing QObject::event,
a catch around a direct call succeeds, while the same catch around
QCoreApplication::sendEvent terminates. A QApplication::notify() override
is therefore not a substitute — it never sees the exception.
Note for review: a segment directory that exists before its TIFFs are
written (a tracer mid-run) is now skipped until complete rather than
registered early. The incremental reload picks it up once written.
Tested: test_segmentation 13/13, including new coverage for missing and
partial payloads. Opening a project with a payload-less segment now
reports "Loaded 0/1 ... Missing: <id>" and keeps running; a segment with
unreadable TIFFs is skipped. Pre-existing unrelated failures in
test_volume_pkg_full and fiber_save_batch_tracker reproduce without these
changes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@JamesDarby345 is attempting to deploy a commit to the scroll Team on Vercel. A member of the Team first needs to authorize it. |
JamesDarby345
marked this pull request as ready for review
July 25, 2026 14:50
Contributor
Author
|
Review by launching the VC3D app and loading a data volume successfully; these fixes target doing that natively on macOS |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two crashes hit while building and running VC3D from source natively on macOS (Apple Silicon, Homebrew LLVM). Both abort the process with an uncaught exception that a handler already in the code should have caught. They're independent and split into one commit each, so either can be dropped.
1.
ld64.lldbreaks exception unwinding in executablesSymptom — opening any volume package:
Why it throws at all, and why that's fine.
CState::setVpkgemits before a volume is loaded, soCWindow::updateAtlasFiberDockscallsresolveLasagnaForVolumewith an id that isn't inloadedVolumes_yet. It throws. That's an expected condition on every project open, and thecatch (...)inupdateAtlasFiberDocksexists precisely to swallow it.Why it aborts. It never reaches that handler. From the crash report:
failed_throwmeans unwinding found no handler, with the handler plainly on the stack.CMakeLists.txtalready documents this exactld64.llddefect and works around it — but only forSHARED_LIBRARYtargets, since it was first observed for cross-dylib throws. With Homebrew clang/lld 22.1.8 the same corruption occurs within theVC3Dexecutable, which stays on lld.Evidence — same objects, same dylibs, only the executable link changed:
ld64.lld22.1.8ldThe fix extends the existing workaround to all targets. Trade-off: some macOS link time for exceptions that work. CI doesn't catch it because it pins an older dependency set whose lld still linked executables correctly — worth knowing that the pin is currently load-bearing.
2. A segment with no tifxyz payload aborts the app
Symptom — after a partially completed Open Data sample download, clicking Reload Surfaces:
The cache held
meta.json+catalog-origin.jsonfor 15 segments but payload for only one.Cause.
Segmentation::canLoadSurface()only checksformat == "tifxyz", so a payload-less directory registers as a usable surface.loadSurface()returns aQuadSurfacewhose TIFFs are read lazily byensureLoaded()on first geometry access — long afterloadSurface()'s try/catch has returned. That first access lands inside a Qt slot:Nothing in that chain catches. The identical error is already survivable one frame over —
ViewerManager's SurfacePatchIndex task wraps its call and logssingle-surface task failed— which is why one segment logs a warning and the next one kills the session.Fix, two layers:
canLoadSurface()also requiresx/y/z.tifto exist. An incomplete directory is simply not loadable, keeping the failure at registration time whereloadSurface()already returnsnullptrand callers handle it.onSurfaceChanged()becomes a thin guard aroundonSurfaceChangedImpl(), so a payload that goes missing after registration (deleted mid-session, corrupt TIFF, unmounted share) logs and skips the surface.Why the guard is inside the slot rather than a global net. I first implemented containment as a
QApplication::notify()override, the usual approach. It cannot work: exceptions don't propagate through Qt's dispatch frames. With a throwingQObject::event:sendEvent)notify)The crash report showed
VCApplication::notifyon the stack with itstryactive while the throw terminated inside QtWidgets frames below it. So that approach was dropped. Practical implication for this codebase: acatchplaced above a Qt frame is decoration — containment must happen before control returns to Qt, which is what the existingViewerManager/LineAnnotationControllerhandlers already do.Testing
test_segmentation: 13/13, including new cases for missing and partial payloads.Loaded 0/1 ... Missing: <id>and keeps running (previously SIGABRT).test_volume_pkg_full(volume attachment) andfiber_save_batch_tracker::emptyBatchCompletesfail — verified pre-existing by stashing these changes, rebuilding clean, and reproducing both.For review
A segment directory that exists before its TIFFs are written — a tracer mid-run — is now skipped until complete rather than registered early. The incremental reload picks it up once written, but that's the intentional behavior change most worth a second opinion.
🤖 Generated with Claude Code