Skip to content

Bridge-level thread-handling contract: per-call safety classification, scoped/controllable internal parallelism, encapsulated global state — safety AND throughput #342

Description

@gsdali

Companion to #341 (NCollection race characterisation, CLOSED), from OCCTReconstruct's contention audit (OCCTReconstruct#175/#309, 2026-07-21).

Rewritten 2026-09-21. The original body's four "measured starting points" were re-measured; two had gone stale and one turned out to be a live documentation defect. What follows is the current state, with the original measurements kept where they still hold so the argument stays auditable.

The problem, unchanged

The thread-safety surface is not knowable from the API. OCCTSerial exists but is opt-in, callers discover unsafe calls one crash at a time (#298, CLOSED, was exactly this: the documented "independent shapes are thread-safe" guarantee was wrong for fillet and chamfer), and the practical downstream response is blanket serialization. Measured cost for one consumer's suite: 4213s serialized against 2066s parallel.

A defined and enforced contract at the bridge is both the safety fix and the speed-up.

Measured state, 2026-09-21

Claim Status
890 OSD_ThreadPool symbols in the bundled kernel still exact
Kernel built WITHOUT TBB (OSD_Parallel_TBB.cxx.o compiled empty) still true
STEP writer mutates Interface_Static process globals still true, now characterised, see below
OCCTBridge_Modeling.mm:1562 hard-codes SetRunParallel(Standard_True) STALE, twice over
OCCTSerialQueue.swift documents no per-call classification was worse than stated, now fixed

The two stale items

The file is gone. OCCTBridge_Modeling.mm was split into 12 OCCTBridge_Modeling_<Bucket>.mm files by #396 (CLOSED). Any plan built from :1562 needs to re-find the site.

The SetRunParallel situation has inverted. It was removed under #367/#369 after appearing to cause silent data corruption. The default is now serial, with SetRunParallel(Standard_False) explicit at OCCTBridge_Modeling_Boolean.mm:4068 and :4163, and a comment at :1071 recording that re-enabling is "very likely safe". So the problem is no longer oversubscription; it is that we leave cores idle. CLAUDE.md tracks re-enabling as a separate open decision.

Checking TBB is worth one warning: a naive nm -C ... | grep -i tbb returns three hits, but two are Vrml_WWWInline::SetBbox* (the substring "tbb" inside "SetBbox") and the third is the OSD_Parallel_TBB.cxx.o object header with no symbols under it. That absence is what "compiled empty" looks like.

What has been delivered since filing

Proposal item 2 is already done for the data-exchange surface, and nobody had written it down. igesMutex() is exactly the "auto-acquire inside the bridge for global-state calls" this issue asks for: 40 bridge entry points (23 STEP, 15 IGES, 2 Document) hold a process-wide recursive mutex for their whole body.

The measurement behind why it must stay is #1403 (OPEN) and its carried patches 0036 through 0041:

TSan reports Named racing globals
Before 178 16
After 37 0

Interface_Static is only part of it. The data-exchange path also routes through a per-format singleton controller and its shared write actor, which is #1403's remaining item and is not fixable by any lock (nine call sites treat the actor as a configuration channel). Upstream research context: OCCT#1179 (CLOSED).

The documentation defect this issue's first bullet led to. OCCTSerialQueue.swift did not merely lack a classification, it stated the opposite of the truth: "Individual bridge calls are NOT auto-locked", in the one file a consumer reads before deciding whether they need OCCTSerial. Fixed in #2073 (MERGED), along with the same claim in its public doc comment and the total absence of thread-safety documentation in Exporter.swift.

What is left, and what closing this requires

  1. Classify every bridge entry point as concurrent-safe / exclusive / global-state, and publish the table. Not started. The DE surface is now effectively global-state-and-handled, so the work is the remaining ~700 entry points. This is the bulk of the issue.
  2. Enforce: auto-acquire for exclusive/global-state calls. Done for DE, unproven elsewhere because step 1 has not run. Interface_Static snapshot/restore is moot: the whole DE surface is serialized instead.
  3. Control internal parallelism. Needs rewriting around the inverted default. The question is now whether and how to re-enable SetRunParallel, plus whether pool sizing should be caller-controllable so N concurrent pipelines can bound total workers instead of N x ncores. OSD_ThreadPool/BOPTools_Parallel: root-cause the cross-caller corruption behind #367 #369 is the decision record.
  4. Throughput payoff. Still follows, and still the point: with a trustworthy concurrent-safe surface, consumers stop blanket-serializing.

Closing criteria

  • A published per-entry-point classification table in docs/thread-safety.md, derived, not hand-maintained (4,056 entries cannot be curated), with a gate that fails when a new bridge entry point is added without a classification. The realistic shape is a script that infers the class from what a function touches (shared caches, process globals, igesMutex()) and a small adjudicated exception list, in the mould of Scripts/census-*.py.
  • Every exclusive/global-state call auto-acquiring, or a documented reason it does not.
  • A decision recorded on SetRunParallel and pool sizing, either way.

Items 1 and 2 are the substance. Item 3 is a decision, not a build. Item 4 is the consequence, not work.

Precedents that still hold: #298 plus patch 0003 (the classify-then-fix loop works), patch 0010 (#319, CLOSED: measured runaway to bounded), and now 0036-0041 (measure, relocate ownership, re-measure). Nothing here needs TBB or upstream changes except where #341's characterisation says so.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority:P2Normaltype:epicLarge effort tracked via sub-issues

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions