Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ suite into these targets (each `Tests/OCCT<Domain>Tests/`, declared in `Package.
- ~~Container-overflow in NCollection on arm64 macOS~~ — **this claim was never characterized and does not hold up.** Investigated for #341 (2026-07-21) using the #298 TSan protocol (minimal-module ThreadSanitizer build of V8_0_0_p1 + all 10 carried patches): `FoundationClasses`+`ModelingData`+`ModelingAlgorithms` stress scenarios (concurrent create/fuse/fillet, independent meshing) are clean except the already-known benign `BOPAlgo_InitMessages` lazy-init race. No NCollection race reproduced anywhere. Re-enabled the 3 suites in `Tests/OCCTStressTests/StressConcurrencyTests.swift` that had been `.disabled()` under this same unevidenced claim (some since ~v0.51.0) — 25/25 clean runs. **A real, different, previously-undetected race was found instead**: `XCAFDoc_ShapeTool::theAutoNaming`, a process-global `static bool` that `RWMesh_CafReader::fillDocument()`, `RWGltf_CafReader::fillDocument()` (a separate near-duplicate override — reachable via OBJ **and** glTF import), and `XCAFDoc_Editor::Expand()` (reentrant — recurses into itself) all save/mutate/restore with zero synchronization; `XCAFDoc_ShapeTool::AddShape` reads the same flag from every one of those and from ordinary unscoped calls too. Same failure class as #298 (unsynchronized global save/modify/restore), but logical/cosmetic (wrong auto-naming, or racy for the plain `bool` itself) rather than geometric. **Fixed upstream in v1.15.5** (`Scripts/patches/0011-*`, xcframework rebuilt): `XCAFDoc_ShapeTool::AutoNamingScope` (RAII, `std::recursive_mutex`-backed) replaces the three ad hoc save/restore call sites, and `theAutoNaming` itself is now `std::atomic<bool>` so unscoped readers (e.g. `AddShape` calls outside any of the three sites) are no longer racing on the raw storage either. Verified via TSan: 0 races across 4 runs (was 9-17/run), zero regression on the #298/independent-meshing scenarios. The interim bridge-side `meshCafMutex()` mitigation shipped in v1.15.4 was removed once this kernel patch shipped — matches the #298 PR1→PR2 pattern. Filed upstream as [OCCT#1387](https://github.com/Open-Cascade-SAS/OCCT/issues/1387) (repro) / [OCCT#1388](https://github.com/Open-Cascade-SAS/OCCT/pull/1388) (fix, draft). See [`Scripts/repro/341-meshcaf/`](https://github.com/SecondMouseAU/OCCTSwift/tree/main/Scripts/repro/341-meshcaf) for the TSan reproducer and full writeup. #341. The two hard crashes (SIGSEGV/SIGABRT) observed empirically in ~2/20 full-suite parallel `swift test` runs during this investigation were filed separately as #344 (SIGSEGV, root-caused below) and #345 (SIGABRT, still uncharacterized — essentially no localizing evidence).
- `XCAFApp_Application::GetApplication()` / `CDF_Directory::Add` — **#344, the SIGSEGV #341 didn't explain.** Confirmed to survive the #341 kernel fix in v1.15.5 (re-ran the parallel `swift test` loop 12× on v1.15.5: 1 more hit, same signature) — a genuinely different, previously-undetected pair of races. `GetApplication()`'s lazy singleton init (`static Handle(XCAFApp_Application) locApp; if (locApp.IsNull()) { locApp = new XCAFApp_Application; }`) is a textbook double-checked-locking-without-locking bug: two threads' first concurrent call can both construct a new instance and race to assign `locApp`. TSan shows this is the dominant defect — it produces multiple concurrently-constructed `XCAFApp_Application` instances, cascading into races across dozens of unrelated destructors as the "losing" instances are torn down mid-flight. Separately, `CDF_Directory::Add`/`Remove`/`Contains` mutate/read `myDocuments` (a plain `NCollection_List`) with zero synchronization — every `CDF_Application` is normally one process-wide instance shared by every caller, so its one `CDF_Directory` receives `Add()` from every document-creating call on every thread, racing on `NCollection_BaseList::PAppend`. The #341 TSan stress never caught either: it builds `TDocStd_Document` directly, bypassing `XCAFApp_Application`/`CDF_Application` entirely — the real bridge path (`OCCTDocumentLoadOBJ` and every other document-producing call) does not. **Fixed in v1.15.6** (`Scripts/patches/0012-*`, xcframework rebuilt): `GetApplication()` folds construction into the static local's initializer (C++11 magic statics, thread-safe exactly once); `CDF_Directory` gets a private mutex guarding `Add`/`Remove`/`Contains`/`Length`/`IsEmpty`/`Last`. Verified via a debug (`-O0 -g`) build with a temporary `SIGSEGV`/`SIGBUS` handler: stock p1 crashes ~50% of runs at 10 threads × 3000 barrier-synchronized rounds, both captured backtraces resolving to `TDocStd_Application::NewDocument -> CDF_Application::Open`; TSan (same minimal-module protocol as #298/#319/#341) goes from 234 race reports to 9, all in `CDF_Directory::Add`/`PAppend` and all showing the same mutex held on both sides of the reported conflict — consistent with a TSan/allocator-recycling artifact (a control program with a trivially-correct mutex pattern shows no such warning under identical flags), not a genuine unaddressed race; the entire `GetApplication()`-driven destructor cascade is gone entirely. Filed upstream as [OCCT#1389](https://github.com/Open-Cascade-SAS/OCCT/issues/1389) (repro) / [OCCT#1390](https://github.com/Open-Cascade-SAS/OCCT/pull/1390) (fix, two commits). **Found during validation of this same fix**: correctly making `GetApplication()` a true singleton means every caller now genuinely shares ONE `TDocStd_Application` instance — surfacing more races on that instance's OTHER unsynchronized state, previously masked by threads sometimes getting different (uncontended) instances. Repeated `swift test` runs hit a SIGTRAP in `Resource_Manager::SetResource` (via `TDocStd_Application::DefineFormat`, itself called by the common `Document.defineAllFormats()` test-setup path) and a SIGSEGV in `TDocStd_Application::ReadingFormats` iterating `CDF_Application::myReaders` concurrently with a writer. `TDocStd_Application::Resources()` has the identical lazy-init bug as `GetApplication()`; `Resource_Manager`'s maps and `CDF_Application::myReaders`/`myWriters` have zero synchronization. Also fixed in v1.15.6 (same patch): a mutex for `Resources()`'s lazy-init, a `std::recursive_mutex` for `Resource_Manager`'s accessors (added an explicit copy constructor too — the new mutex broke `ShapeProcess_Context.cxx`'s existing `new Resource_Manager(*sRC)` thread-safety workaround, whose own comment already acknowledged this exact defect: *"calling of SetResource() for one object in multiple threads causes race condition"*), and a mutex for `myReaders`/`myWriters`. 0/12 further `swift test` runs of `OCCTXCAFTests` reproduce either crash after the fix. A THIRD, architecturally different crash surfaced in the same validation (`BinLDrivers_DocumentStorageDriver::Write` corrupting a shared, cached, non-reentrant storage-driver instance under concurrent `Save`/`SaveAs` of the same format) — a shared worker object, not a container needing a lock, so the kernel fix needs its own TSan investigation; filed separately as #349. It was severe enough on its own (~60% crash rate in `OCCTXCAFTests` alone once the two races above stopped masking it) that v1.15.6 ships an **interim bridge-side mitigation** for it too: `ocafStoreMutex()` (`OCCTBridge_Document.mm`) serializes `OCCTDocumentSaveOCAF`/`OCCTDocumentSaveOCAFInPlace`/`OCCTDocumentLoadOCAF` — the same #298/#341 PR1→PR2 pattern (bridge mutex now, kernel fix later). 0/12 further `swift test` runs of `OCCTXCAFTests` crash after this mitigation (some pre-existing, unrelated test-fixture flakes remain — hardcoded non-unique temp file paths across parallel `OCAF Save/Load` tests yielding `.alreadyRetrieved`, and the already-known `Issue173AssemblySTEPTests` flake — neither a kernel bug). See [`Scripts/repro/344-cdf-directory/`](https://github.com/SecondMouseAU/OCCTSwift/tree/main/Scripts/repro/344-cdf-directory) for the full writeup. #344.
- **Uncaught-exception SIGABRT — #345, the companion crash to #344.** #345 was filed alongside #344 from the same investigation with "essentially no localizing evidence" (just `exited with unexpected signal code 6`, no test name, no backtrace). An audit (prompted by #344's own finding that fixing one race exposes the next) turned up a plausible, distinct root cause: OCCT's `gp_Dir` constructor throws `Standard_ConstructionError` for a zero-length (or near-zero) direction/normal vector (`Libraries/occt-src/src/FoundationClasses/TKMath/gp/gp_Dir.hxx`); so does `Geom_Direction`'s. **49 public bridge functions** across `Sources/OCCTBridge/src/{OCCTBridge_Curve3D,OCCTBridge_Geom2d,OCCTBridge_Modeling,OCCTBridge_Spatial,OCCTBridge_Surface,OCCTBridge_Topology,OCCTBridge_Visualization}.mm` constructed `gp_Dir`/`gp_Ax1`/`gp_Ax2`/`gp_Ax3`/`Geom_Direction` directly from caller-supplied doubles (or called a `Geom_Surface`/`Geom_Curve` `D0`/`D1`/`D2` derivative evaluator, or a `GeomEval_*Surface` constructor — same throw risk for degenerate radius/amplitude/omega) with **no try/catch anywhere in the call chain** — e.g. `OCCTSurfaceD1`/`OCCTSurfaceD2` had none, right next to `OCCTSurfaceGetNormal` two lines below, which already did (an easy-to-miss inconsistency). An uncaught C++ exception crossing the bridge's `extern "C"`-ish boundary into Swift-generated call frames has no matching unwind personality routine — a guaranteed `std::terminate()` → `abort()` (SIGABRT), and it would leave almost no diagnostic trail, matching #345's profile exactly. **Fixed**: wrapped all 49 in `try { ... } catch (...) { <safe fallback> }` matching each file's existing idiom; the 3 functions returning a `_Nonnull` pointer (`OCCTAxis1PlacementCreate`, `OCCTAxis2PlacementCreate`, `OCCTOBBCreate`) fall back to constructing a valid default axis instead of `nullptr`, since returning null from a `_Nonnull` contract would just relocate the crash to the next dereference. Two confirmed false positives were left untouched: `computePlaneForPoints` (`OCCTBridge_AIS.mm`) and `buildTrsf3D` (two separately-defined `static` helpers, one each in `OCCTBridge_Curve3D.mm`/`OCCTBridge_Surface.mm`) are both already protected by a `try` in their sole caller. New regression tests (`Tests/OCCTStressTests/StressNullInvalidTests.swift`: `mirrorAxisZeroDirection`, `mirrorPlaneZeroNormal`, `geomDirectionZeroVector`) exercise a zero vector through the public Swift API. **Validation**: 70 additional full-suite `swift test` runs (4419-4422 tests each, ~309,540 individual test executions) — zero SIGABRT recurrence, zero crashes of any kind (a handful of already-known, unrelated assertion-level test-fixture flakes remain). This is a bridge-only fix (no OCCT kernel change, no xcframework rebuild) — not an OCCT bug, so nothing filed upstream. #345's own bar for confident closure was "100+ runs with no recurrence"; 70 clean runs plus a fix matching the exact crash mechanism is short of that literal bar but is the strongest evidence gathered to date — recommend closing #345, reopen if it recurs.
- `PCDM_StorageDriver`/`PCDM_Reader` (backs `CDF_Application::WriterFromFormat`/`ReaderFromFormat`) — **#349, the third crash found validating #344.** `CDF_Application` caches one storage/retrieval driver instance per format and hands the same instance back to every `Store()`/`Retrieve()` call for that format, including concurrently from different threads/documents. But `BinLDrivers_DocumentStorageDriver` (and every other format's driver, structurally) is not reentrant: `Write()`/`Read()` mutate instance-level scratch state (`myRelocTable`, `myTypesMap`, and more) with zero synchronization, so two threads' concurrent `Write()` calls corrupt each other — reliable SIGSEGV (`BinMDF_ADriverTable::AssignIds` on a torn `myTypesMap`, reached via `BinLDrivers_DocumentStorageDriver::Write -> FirstPass -> CDF_StoreList::Store -> TDocStd_Application::SaveAs`). TSan (same minimal-module protocol as #298/#319/#341/#344) confirms directly: 136 race warnings + a live SIGSEGV in one run (8 threads × 25 rounds) on stock kernel. **Fixed in v1.15.9** (`Scripts/patches/0014-*`, xcframework rebuilt): `PCDM_StorageDriver`/`PCDM_Reader` each get a `mutable std::mutex` + `Mutex()` accessor — every concrete format driver subclass inherits the guard for free, zero subclass changes needed — held at the three call sites that invoke a cached, possibly-shared driver (`CDF_StoreList::Store`, `CDF_Application::Retrieve`, `CDF_Application::Read`). Considered making the driver itself reentrant (eliminating the shared scratch state) but rejected as impractical: TSan shows essentially the entire driver object is scratch state for one `Write()` call, plus a nested shared `BinMDF_ADriverTable` with its own internal mutation — fixing that properly would mean a sweeping signature change to every format driver's private helpers, out of proportion to a "minimal, surgical" upstream PR. Verified: TSan race warnings 136 → 0 (confirmed again at 10×200), crash → clean exit across repeated runs; full production xcframework rebuild + `swift test --filter OCAFSaveLoadBinaryTests`/`OCCTXCAFTests` clean, 3× full `swift test` (4423 tests) clean. The interim bridge-side mitigation (`ocafStoreMutex()`, shipped v1.15.6) stays in place — same PR1→PR2 pattern as #298/#341/#344. **Found during validation of this fix**: `CDM_Application::myMetaDataLookUpTable` (a plain, unsynchronized `NCollection_DataMap`, one per `CDM_Application`) races too — same failure class as `CDF_Directory::myDocuments` (#344) and `theAutoNaming` (#341), out of scope for #349, filed separately as #353. See [`Scripts/repro/349-ocaf-driver-reentrancy/`](https://github.com/SecondMouseAU/OCCTSwift/tree/main/Scripts/repro/349-ocaf-driver-reentrancy) for the full writeup. Filed upstream as [Open-Cascade-SAS/OCCT#1393](https://github.com/Open-Cascade-SAS/OCCT/issues/1393) (repro) / [OCCT#1394](https://github.com/Open-Cascade-SAS/OCCT/pull/1394) (fix, CI green on all platforms). #349.
- `LocOpe_SplitDrafts` throws on incompatible geometry — always wrap `Perform()` in try-catch in bridge
- `BRepOffsetAPI_ThruSections` (loft) SIGSEGV'd (null deref, "Address 8") on mismatched closed profiles — `BRepFill_CompatibleWires::SameNumberByPolarMethod` over-advanced an unguarded correspondence-list iterator. It's an OS signal, so the bridge `catch(...)` cannot save it. **Fixed upstream in OCCT 8.0.0p1** (Open-Cascade-SAS/OCCT#1298, OCCTSwift #176/#178); the previously-carried `Scripts/patches/0001-*` was dropped — the current p1 xcframework has the guard natively (regression test "Loft polar-method SIGSEGV regression (#176)" passes against it). Note: `OCC_CATCH_SIGNALS` is inert in our build (no `OCC_CONVERT_SIGNALS`) — do not rely on it for signal safety; OS signals raised inside OCCT (e.g. #234) are still uncatchable in-process.
- `ShapeAnalysis_FreeBounds` (backs `Shape.freeBoundsClosedWires`/`freeBoundsClosedCount`/`freeBoundsOpenWires`, and `Shape.freeBounds`) used to SIGSEGV (uncatchable) on certain shapes with multiple free-boundary components — isolated via AddressSanitizer to `connectWiresToWiresImpl`'s empty-input early return (`ShapeAnalysis_FreeBounds.cxx`) leaving its `owires` out-parameter uninitialized (null), which later crashes `NCollection_HSequence::Append`. Minimally reproducible with just two disjoint planar faces in one compound; not a simple loop-count threshold (150+ loops can be fine, 2 can crash) — depends only on whether any single component's boundary closes with zero edges left over. **Fixed upstream in v1.12.6** (`Scripts/patches/0004-*`, xcframework rebuilt): `connectWiresToWiresImpl` now initializes `owires` to an empty sequence before its early return. #310, upstream repro filed as Open-Cascade-SAS/OCCT#1376, fix as [OCCT#1377](https://github.com/Open-Cascade-SAS/OCCT/pull/1377); sibling `Standard_OutOfRange` bug in the same file, OCCT#1330, was a separate, unrelated defect in the same function — also now carried, see the `0007` entry below.
Expand Down
11 changes: 6 additions & 5 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ let occtTarget: Target = useLocalBinary
name: "OCCT",
path: "Libraries/OCCT.xcframework"
)
// v1.15.8 rebuild: OCCT 8.0.0p1 + our carried patches — 0001 (ShapeFix_Face guard, #263),
// v1.15.9 rebuild: OCCT 8.0.0p1 + our carried patches — 0001 (ShapeFix_Face guard, #263),
// 0002 (backport of upstream OCCT#1334, #280), 0003 (fillet TopOpeBRep thread_local, #298),
// 0004 (ShapeAnalysis_FreeBounds owires init, #310), 0005 (ShapeFix_Face null-Context guard
// in FixPeriodicDegenerated, #317), 0006 (BRepGProp_EdgeTool adaptor NbPoles, #318), 0007
Expand All @@ -42,14 +42,15 @@ let occtTarget: Target = useLocalBinary
// (Intf_Interference O(1) tangent-zone lookup + checkpointed breaker, #319), 0011
// (XCAFDoc_ShapeTool::AutoNamingScope, #341), 0012 (XCAFApp_Application::GetApplication/
// TDocStd_Application::Resources lazy-init races + CDF_Directory/Resource_Manager/
// CDF_Application reader-writer map synchronization, #344), and 0013
// (ShapeUpgrade_UnifySameDomain null-pcurve dereference guards, #348).
// CDF_Application reader-writer map synchronization, #344), 0013
// (ShapeUpgrade_UnifySameDomain null-pcurve dereference guards, #348), and 0014
// (PCDM_StorageDriver/PCDM_Reader driver-instance reentrancy mutex, #349).
// Bump BOTH url and checksum whenever the xcframework is rebuilt, or URL-resolving consumers
// silently keep the previous kernel while local sibling builds get the new one.
: .binaryTarget(
name: "OCCT",
url: "https://github.com/SecondMouseAU/OCCTSwift/releases/download/v1.15.8/OCCT.xcframework.zip",
checksum: "b1e72863654a2e979f0218b720459d48608ab5f25b52c641899ee7c1618b2b92"
url: "https://github.com/SecondMouseAU/OCCTSwift/releases/download/v1.15.9/OCCT.xcframework.zip",
checksum: "61937268c17b19a8628d7494cb05151d17f0c2908160b8e0b999d9588aa74e33"
)

// OCCTBridge is 16 Objective-C++ files / ~62K lines wrapping the OCCT header tree; SwiftPM recompiles
Expand Down
Loading