BUG: Avoid concurrent map insertion in Compute Triangle Face Ids #1772

Problem and scope

Compute Triangle Face Ids can modify a shared map while other worker threads read it. Both the in-memory and OOC algorithms use this code. Handle the correction on a separate branch so the in-memory fix can be reviewed and merged independently of the OOC audit branch.

This is a source-confirmed unsafe access path. No crash, incorrect output or measured occurrence rate has been reproduced. It is reachable with Randomize Face IDs disabled; the randomization/table-alignment concern is separate.

Cause

Source is pinned to the published head of PR #1764, 9a28a90f0f7a5d42fb5f17f36c84597b640fcac9.

• SharedFeatureFaceImpl retains a mutable map reference and uses m_FaceSizeMap[faceId64] to obtain each triangle count.
• Both algorithms create a reserved (0,0) metadata row but populate the map only for label pairs actually found in the triangles. The OOC setup/grouping and resident setup/grouping have the same behavior.
• Both launch the worker over all metadata rows, including row zero, with parallel processing enabled: OOC caller, resident caller.
• When the input contains no (0,0) pair, row zero uses operator[] on a missing key. That inserts a zero-valued node while another worker may be searching the same map. Map insertion changes tree links used by other lookups.

The normal range wrapper has no small-input threshold. Actual overlap depends on available threads and scheduling. One successful run, or even many successful runs, does not establish race freedom.

Conditions to cover

Use valid triangle geometry with at least one real boundary, multiple worker threads, and no input (0,0) label pair. For a small distinct-count fixture, use these six pairs:

(7,3), (-1,4), (9,11), (3,7), (4,-1), (7,3)

Expected IDs without randomization are {1,2,3,1,2,1}; metadata rows are {(0,0),(3,7),(-1,4),(9,11)} with counts {0,3,2,1}. The reserved row is the missing-key case. A separate real-(0,0) control is also necessary.

The OOC worker writes ordinary resident temporary buffers and bulk-publishes them after the parallel work joins. This finding concerns the map; it is not a general claim that arbitrary DataStore implementations support parallel access.

Recommended correction and alternatives

Make the worker's map reference const and use a non-inserting lookup. Use the existing count when present and zero when absent. Keep grouping, ID assignment, parallel scheduling and cancellation behavior unchanged.

This preserves the missing-key numeric result without changing the shared structure. It adds no lock or forced serialization. No performance measurement is claimed.

Important compatibility details:

• If an actual (0,0) pair exists, preserve its current count/ID behavior, including the reserved row's current lookup result. Do not unconditionally zero that row as part of this correction.
• Do not seed key zero before grouping: that can change first-encounter ID assignment for a real (0,0) pair.
• Serial metadata population or a mutex could also remove the race, but introduces a performance/synchronization tradeoff that the const lookup avoids.

Acceptance checks

• The parallel worker cannot insert into or otherwise mutate the shared map.
• No-(0,0) and real-(0,0) cases preserve complete IDs, label rows and counts.
• Cover both resident algorithm scenarios and actual HDF5 inputs/outputs; keep bounded storage behavior.
• Preserve cancellation boundaries, input values and metadata publication/error behavior.
• Keep multithreaded execution in ordinary success tests. Use race detection where available; do not silently serialize the test to conceal the defect.
• Complete the deferred block/tail and transfer-failure coverage after the shared correction is available.

No production change for this finding has been made on the OOC audit branch. Historical origin and randomization correctness are not established by this report.