Fix concurrent map races in Statelet, collector, and session cache

[alexis779]

Summary

• Locks shared maps that could fatal with concurrent map iteration and map write (and the read/write variant) under parallel reads.
• Statelet._columnNames is the production path: CloneForSummary ranged the map while Add wrote it. Collector parent indexes, PredicateRegistry, session cache JSON, and Types.Lookup had the same shape.
• Each site has a 3s stress test that crashed on the unlocked tree and now survives.

Fixes #107.

Test plan

• go test -timeout 10s -count=1 -run TestStateletCloneForSummaryConcurrentColumnNames ./view/ — crashed before the lock, passes after 3s
• go test -timeout 10s -count=1 -run TestCollectorConcurrentParentValuePositions ./view/
• go test -timeout 10s -count=1 -run TestPredicateRegistryConcurrentLookupAndAdd ./view/extension/
• go test -timeout 10s -count=1 -run TestSessionMarshalJSONConcurrentCachePut ./service/session/
• go test -timeout 10s -count=1 -run TestDataUnitConcurrentSliceIndex ./service/executor/expand/
• go test -timeout 10s -count=1 -run TestTypesConcurrentLookupAndPut ./view/state/
• Existing collector/bootstrap tests still pass

[alexis779]

Review feedback

High: view/state.go:194 still races. CloneForSummary() copies the embedded QuerySelector before locking columnNamesMu, and that embedded struct includes Fields / Columns. Meanwhile Statelet.Add() mutates Fields / Columns under the lock. The PR’s own race test still fails under go test -race.

Fix: do not assign QuerySelector: s.QuerySelector before the lock; copy scalar fields separately, and clone Fields / Columns while holding the same lock.

Medium: view/collector.go:887 still reads/writes r.valuePosition directly in ViewMetaHandler without the new collector index lock. Most new accessors use parentValuesPositions(...), but this path still bypasses it. If meta/summary binding overlaps with parent indexing, this remains a race candidate.

Validation

Non-race tests pass on the refreshed PR branch:

go test ./service/executor/expand ./service/session ./view ./view/extension ./view/state -count=1

Race-focused tests fail:

go test -race ./service/executor/expand ./service/session ./view ./view/extension ./view/state -run 'Concurrent|CloneForSummary|Registry|MarshalJSON|Types' -count=1

The concrete failure is TestStateletCloneForSummaryConcurrentColumnNames, reporting Statelet.Add() vs Statelet.CloneForSummary() on Fields / Columns.

[alexis779]

How the review feedback was addressed

Pushed in d104f61b on this branch.

High — CloneForSummary() race on Fields / Columns

CloneForSummary() no longer assigns the embedded QuerySelector (QuerySelector: s.QuerySelector) before taking columnNamesMu. That copy included Fields and Columns, which Statelet.Add() mutates under the same lock.

The clone now:

• copies scalar selector fields (OrderBy, Offset, Limit, Page, Criteria) without touching the shared slices
• clones _columnNames, Fields, and Columns while holding columnNamesMu

That is the same lock Add() uses, so the previous Add vs CloneForSummary race on those slices is gone.

Medium — ViewMetaHandler bypassing the collector index lock

ViewMetaHandler no longer reads or writes r.valuePosition unlocked. The lookup now:

• takes lockIndex() / unlockIndex() (the same collector index lock as the other accessors)
• copies the []int positions before using them after the unlock

This path is on the collector’s own index (not the parent map), so it uses lockIndex rather than parentValuesPositions(...). Meta/summary binding no longer races with concurrent parent indexing.

Re-validation

Non-race:

go test ./service/executor/expand ./service/session ./view ./view/extension ./view/state -count=1

Race-focused (now passing, including TestStateletCloneForSummaryConcurrentColumnNames):

go test -race ./service/executor/expand ./service/session ./view ./view/extension ./view/state -run 'Concurrent|CloneForSummary|Registry|MarshalJSON|Types' -count=1

[alexis779]

Review feedback

Medium: view/collector.go:235 and view/collector.go:252 lock while finding/building valuePosition / compositeValuePosition, but return the mutable map after unlocking. Several callers then read from that returned map outside the lock, while writers like indexValueToPosition mutate under the same lock. If parent indexing and child relation matching can overlap, this still leaves unlocked map reads.

Safer shape: return copied positions for a specific key under lock, like the updated ViewMetaHandler now does.

Resolved from prior review

• The previous Statelet.CloneForSummary race is fixed. It no longer copies the embedded QuerySelector with Fields / Columns before locking.
• The previous ViewMetaHandler direct map access is fixed. It now copies positions under lock.

Validation

Latest PR head before this follow-up:

d104f61b Clone selector slices under lock during summary copy.
bd33e373 Lock shared maps that raced under concurrent reads.

Tests passed on the refreshed PR worktree:

go test ./service/executor/expand ./service/session ./view ./view/extension ./view/state -count=1
go test -race ./service/executor/expand ./service/session ./view ./view/extension ./view/state -run 'Concurrent|CloneForSummary|Registry|MarshalJSON|Types' -count=1

I would not block on the old Statelet issue anymore. The remaining question is whether collector parent-index reads can truly overlap with parent-index writes in production execution. If yes, the returned-map pattern should be tightened before merge.

[alexis779]

How the review feedback was addressed

Pushed in ec56fb20 on this branch.

Medium — unlocked reads of returned parent-index maps

parentValuesPositions and parentCompositePositions no longer return the live valuePosition / compositeValuePosition maps after unlocking.

Callers that needed positions for one key now go through:

• parentPositionsFor(ns, column, key) — copies []int under the parent index lock
• parentCompositePositionsFor(relation, key) — same for composite keys

ParentPlaceholders still needs the key set (not a single key). That path uses parentPositionKeys, which copies the keys while the same lock is held.

Writers such as indexValueToPosition already mutate under that lock, so child relation matching no longer ranges or looks up the live maps outside it. TestCollectorConcurrentParentValuePositions now overlaps those writes with key lookups and key copies.

Re-validation

go test ./service/executor/expand ./service/session ./view ./view/extension ./view/state -count=1
go test -race ./service/executor/expand ./service/session ./view ./view/extension ./view/state -run 'Concurrent|CloneForSummary|Registry|MarshalJSON|Types' -count=1

Both passed, including TestCollectorConcurrentParentValuePositions.