fix(#341): kernel patch for XCAFDoc_ShapeTool::theAutoNaming race (v1.15.5)#347
Merged
Conversation
….15.5) Replaces v1.15.4's bridge-side meshCafMutex() mitigation with the real kernel fix (#298 PR1->PR2 pattern). Auditing every internal caller of theAutoNaming turned up two more independent save/modify/restore sites beyond RWMesh_CafReader:: fillDocument(): a separate near-duplicate override in RWGltf_CafReader:: fillDocument(), and XCAFDoc_Editor::Expand() (which recurses into itself). Verifying the bridge mutex under TSan with the mutex removed also surfaced a second problem: an unscoped XCAFDoc_ShapeTool::AddShape call still reads the raw bool with no synchronization at all. Scripts/patches/0011-XCAFDoc_ShapeTool-AutoNamingScope-341.patch: a new RAII XCAFDoc_ShapeTool::AutoNamingScope (recursive_mutex-backed, held for its entire lifetime) replaces all three ad hoc save/restore sites; theAutoNaming itself is now std::atomic<bool> so unscoped readers are no longer racing on the raw storage either. Verified via TSan: 0 races across 4 runs (was 9-17/run), zero regression on #298/independent-meshing scenarios. OCCT.xcframework and OCCTBridge.xcframework (meshCafMutex removed) both rebuilt. Filed upstream as OCCT#1387 (repro) / OCCT#1388 (fix, draft PR, CI green on macOS/Linux/Windows builds + all GTest platforms as of this commit). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
Follow-up to #343/v1.15.4. Replaces the bridge-side
meshCafMutex()mitigation with the realkernel fix, matching the #298 PR1→PR2 pattern.
theAutoNaming(not just the one originally found) turned uptwo more independent save/modify/restore sites: a separate near-duplicate override in
RWGltf_CafReader::fillDocument(), andXCAFDoc_Editor::Expand()(which recurses into itselfwhile the dance is in flight).
isolation) surfaced a second, narrower problem: an unscoped
XCAFDoc_ShapeTool::AddShapecallstill reads the raw
boolwith no synchronization at all — independent of the "logical"interleaving bug.
Scripts/patches/0011-XCAFDoc_ShapeTool-AutoNamingScope-341.patch:XCAFDoc_ShapeTool::AutoNamingScope(new RAII helper,
std::recursive_mutex-backed, held for its entire lifetime) replaces all threead hoc call sites;
theAutoNamingitself is nowstd::atomic<bool>.regression on
create_fillet_boolean(SheetMetal.Builder.build silently returns an empty solid under parallel test execution (8/10 runs) — contradicts the documented "independent shapes are thread-safe" guarantee #298) / independent-meshing scenarios.OCCTBridge_IO.mm'smeshCafMutex()(all 12 lock sites + the mutex itself) removed as redundant.OCCT.xcframework(all 3 core slices) andOCCTBridge.xcframeworkrebuilt;Package.swiftURLs/checksums bumped to v1.15.5 — needs the matching release assets attached.
(repro); fix submitted as OCCT#1388 (draft,
clang-format ✅, non-ASCII check ✅, macOS/Linux/Windows builds ✅, all GTest platforms ✅ as of
this PR).
post-fix full-suite parallel runs, confirming it's independent of the AutoNaming race this PR
fixes.
Test plan
swift buildclean (source bridge andOCCTSWIFT_BRIDGE_PREBUILT=1, against the rebuiltOCCT.xcframework)Issue341MeshCafThreadSafetyTestsand the re-enabledStressConcurrencyTestspassswift test(parallel) × 12 runs post-fix: 9 clean, 2 hit the unrelated pre-existingSweep: ShapeFix/Sewing run outside the caller's progress range in the Robust *Progress import paths (#286 follow-up) #300 timing-flake, 1 hit the unrelated pre-existing Uncatchable SIGSEGV in parallel swift test run, right after concurrent OBJ imports — possibly related to #341, unconfirmed #344 crash — no new failure modes
theAutoNamingraces with the kernel patch alone🤖 Generated with Claude Code