fix(#298): fix fillet reentrancy in the kernel; drop the serialization lock#308
Merged
Conversation
…n lock The real fix for #298 lands in the pinned OCCT, replacing the v1.12.1 bridge lock. Root cause (found with ThreadSanitizer): BRepFilletAPI_MakeFillet reconstructs its result solid via OCCT's legacy TopOpeBRepBuild engine, which passed state between methods through a file-scope static, STATIC_SOLIDINDEX (SplitSolid sets 1/2, FillSolid reads it back to pick the operand). Concurrent fillets on independent shapes clobbered each other's flag, mis-classifying faces -> wrong-but-plausible solid that fails BRepCheck. NOT the BlendFunc scratch the v1.12.1 notes suspected (those race benignly). - Scripts/patches/0003: converts the fillet-path statics to thread_local (STATIC_SOLIDINDEX + STATIC_lastVPind functional; BlendFunc_ConstRad/EvolRad and ChFi3d_Builder checkcurve scratch benign, for TSan-clean). Upstreamed as Open-Cascade-SAS/OCCT#1374. - xcframework rebuilt with 0003; Package.swift url+checksum bumped to v1.12.3. - occtFilletMutex and its 16 bridge guard sites removed — fillet/chamfer are reentrant in the kernel now, so concurrent builds run in parallel again. - Issue298FilletThreadSafetyTests kept; passes with the lock removed. Verified: pure-C++ 8-thread stress 0/1600 invalid (was ~15-20%), TSan-clean on the fillet path; the Swift regression + Modeling/Thread suites pass against the patched binary with the lock gone. Co-Authored-By: Claude Opus 4.8 (1M context) <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
1 participant
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.
Completes #298. v1.12.1 stopped the corruption by serialising every 3D fillet/chamfer build behind a bridge mutex (
occtFilletMutex); this replaces that with the real kernel fix and removes the lock, so concurrent fillet/chamfer run in parallel again.Root cause (ThreadSanitizer)
BRepFilletAPI_MakeFilletreconstructs its result solid through OCCT's legacyTopOpeBRepBuildengine (ChFi3d_Builder::Compute→TopOpeBRepBuild_HBuilder::MergeSolid→TopOpeBRepBuild_Builder::SplitSolid), which passed state between methods through a file-scopestatic,STATIC_SOLIDINDEX:SplitSolidsets it to 1/2 to tellFillSolidwhich operand it is splitting, andFillSolidreads it back to pick the operand shape. Two fillet builds on independent shapes on separate threads clobbered each other's flag, soFillSolidmis-classified faces and returned a wrong-but-plausible solid (one solid, positive volume, failsBRepCheck). This is not theBlendFuncscratch the v1.12.1 notes first suspected — those race benignly.Fix
Scripts/patches/0003converts the fillet-path statics tothread_local(each thread keeps its own copy; single-thread behaviour unchanged):STATIC_SOLIDINDEX+STATIC_lastVPind(functional), plusBlendFunc_ConstRad/EvolRad+ChFi3d_Buildercheckcurvescratch (benign, converted so the path is TSan-clean). Upstreamed as Open-Cascade-SAS/OCCT#1374.Package.swifturl+checksum bumped to v1.12.3.occtFilletMutexremoved (16 bridge guard sites) — the kernel is reentrant now.Verification
Issue298FilletThreadSafetyTestspasses with the lock removed;OCCTModelingTests(409),OCCTThreadTests(55) pass against the patched binary.Binary release — remote SPM consumers pick up the rebuilt lock-free kernel via the new url + checksum.
🤖 Generated with Claude Code