OSD_ThreadPool/BOPTools_Parallel: root-cause the cross-caller corruption behind #367

[gsdali]

Summary

Follow-up investigation flagged in #367 (fixed, closed) — that issue removed the trigger
(OCCTShapeFuseMulti's SetRunParallel(true)) but did not root-cause the underlying mechanism.
Filing this to track that separately, since it's a materially different scale of investigation
than anything else in this project's TSan series so far.

What #367 established

Two independent, unrelated top-level BRepAlgoAPI_BuilderAlgo::Build() calls, each requesting
internal parallelism (SetRunParallel(true)), submit work to the same process-wide
OSD_ThreadPool::DefaultPool(). Under concurrent load this was 100% reproducible data
corruption, not a rare interleaving: 400/400 operations produced wrong results (27 faces instead
of the correct 13), plus 237 TSan race reports across foundational topology code (TopoDS_Builder::Add,
TopExp_Explorer, BRep_Tool::Range, BOPTools_AlgoTools::MakeSplitEdge).

The clearest captured trace: thread T1's own top-level Build() call allocates a vertex via its
own BOPAlgo_PaveFiller. Thread T9 — a worker spawned by a different, unrelated top-level caller
(T4)'s OSD_ThreadPool dispatch — reads and writes that same memory. Worker threads from one
caller's dispatch end up processing another caller's data.

Reproducer: Scripts/repro/342-boolean-ops/occt_342_boolean_stress.cpp, fuse_multi_parallel
scenario.

Why this is a bigger investigation than #298/#341/#344/#349/#353/#361

Every prior TSan finding in this project was a single unsynchronized static/global variable (or a
shared instance that should have been per-object) in a narrow, specific class — findable by reading
that one class's source and fixable by adding a mutex or relocating the state. This finding points
at OSD_ThreadPool/BOPTools_Parallel — OCCT's own shared-pool parallel-dispatch infrastructure,
used internally by many algorithms, not just booleans. Properly root-causing it likely needs
reading OSD_ThreadPool.cxx's actual job-queue/dispatch implementation (job submission, worker
wake-up, completion tracking) to find the real mechanism, not pattern-matching against the prior
fixes' template.

Open questions

• Is OSD_ThreadPool::DefaultPool() fundamentally unsafe for concurrent independent submitters
  (multiple unrelated top-level operations dispatching to it at the same time), or is this specific
  to how BOPTools_Parallel/BOPAlgo_PaveFiller use it (e.g. a job-completion/context-scoping bug
  in the caller, not the pool itself)?
• Does this affect anything else in the bridge that triggers internal OCCT parallelism —
  BRepMesh_IncrementalMesh's InParallel option, BRepCheck_Analyzer::SetParallel, or any other
  SetRunParallel-style opt-in — or is it specific to BRepAlgoAPI_BuilderAlgo/BOPAlgo_PaveFiller?
• Is this already-known/reported upstream (search Open-Cascade-SAS/OCCT's issue tracker before
  assuming it's novel)?

Priority

Not blocking anything currently shipping (#367's immediate fix removed the only bridge call site
that triggered it). Pick up as a dedicated TSan investigation when there's time for a genuinely
open-ended one, following the established minimal-module protocol
(FoundationClasses+ModelingData+ModelingAlgorithms) plus direct reading of
OSD_ThreadPool.cxx/OSD_Parallel.hxx/BOPTools_Parallel.hxx.

[gsdali]

Investigation checkpoint — stopping here for now, recording state so a future session (or anyone else) doesn't have to re-derive this. Full trail with code excerpts in Scripts/repro/342-boolean-ops/README.md.

Ruled out: OSD_ThreadPool itself

Read OSD_ThreadPool.hxx/.cxx in full. The pool's own design intent is documented explicitly — concurrent Launchers on the same pool are supposed to degrade safely (lock whatever's free, fall back toward single-threaded), not corrupt anything. Checked the actual mechanism:

• EnumeratedThread::Lock()/Free() use a genuine atomic exchange — two concurrent Launchers can never both lock the same EnumeratedThread.
• Standard_Condition is a real memory fence (proper std::mutex + std::condition_variable, not a spin-flag).
• Launcher::perform()'s wait() → WaitIdle() genuinely blocks until each worker has finished its job and nulled myJob, before Release()/Free() lets another Launcher reclaim that thread.

Empirically confirmed clean: wrote occt_369_threadpool_isolation.cpp — OSD_ThreadPool::Launcher used directly, zero BOPAlgo/geometry involved, N concurrent top-level callers each writing/reading a private per-call buffer slot (cross-checked for foreign writes). 10 threads × 300 iterations (3000 ops): 0 errors, 0 TSan races.

Narrowed to: BOPTools_Parallel/BOPAlgo_PaveFiller's use of the pool

Read BOPTools_Parallel.hxx (the direct consumer, via ContextFunctor2). Checked the obvious candidates — the per-Launcher context array, the thread-index renumbering, the self-thread context binding, the atomic data-index range — none looked flawed on inspection, but this isn't conclusive; the actual corrupted object from the original trace (a TopoDS_Vertex) is allocated/consumed several layers below this file, inside BOPAlgo_PaveFiller/BOPDS_DS/IntTools_Context, none of which have been read yet.

Suggested next steps (see README for full detail)

1. Extend the isolation harness to use ContextFunctor2 directly with a toy solver, to confirm whether the bug is in BOPTools_Parallel.hxx itself or further down.
2. If narrowed further, read BOPAlgo_PaveFiller::Prepare()/MakeSplitEdges() end to end for anything shared across BOPAlgo_PaveFiller instances that should be per-instance.
3. Check whether NCollection_BaseAllocator::CommonBaseAllocator() (used to construct each per-thread IntTools_Context) is itself safe for concurrent use by unrelated top-level calls — could explain the breadth of the 237 TSan reports better than a narrow indexing bug.
4. Check whether this is already known upstream before assuming it's novel.

Not blocking anything — #367's fix already removed the only trigger. Picking this back up needs a genuinely open-ended investigation session, not a quick follow-up.