ILTypeBuilder lazy member-wrapper caches may race under parallel F# compilation

[sergey-tihon]

Detected inside SwaggerProvider project fsprojects/SwaggerProvider#332

After upgrading from 57d2c4f92758078fbce31976325e7661fa737104 to 32fd6037e053d5d7ed532b84b836eb92fc8784b3 (which added lazy caching for GetConstructors/GetMethods/etc. in ILTypeBuilder), intermittent FS3033 errors appear in SwaggerProvider's CI when the F# compiler invokes type providers from parallel compilation threads:

• Ubuntu: InvalidOperationException: Operations that change non-concurrent collections must have exclusive access. A concurrent update was performed on this collection and corrupted its state. — CI run 23313927434
• Windows: NullReferenceException — CI run 23350007356

Hypothesis: The lazy values ctorDefs/methDefs/etc. are defined per ILTypeBuilder instance and bound to this. The F# compiler can call GetConstructors(bf)/GetMethods(bf) on the same ILTypeBuilder from multiple threads when compiling a project file that has multiple type-provider instantiations. If two threads call ctorDefs.Force() while inp.Methods.Entries is being iterated, the race on the underlying mutable inp data structure produces the above exceptions.

The previous code (57d2c4f9) created a fresh array on each GetConstructors call (no shared state), which avoided this race.

Repro project: fsprojects/SwaggerProvider (see PR #331).
Affected SDK commit: 32fd6037 (unchanged through 821de018).

[sergey-tihon]

/repo-assist plz take a look

[github-actions]

✅ Repo Assist completed successfully!

[github-actions]

Pull request created: #482

Generated by Repo Assist

[github-actions]

🤖 This is an automated response from Repo Assist.

Thanks for the detailed report and repro, @sergey-tihon!

Root cause confirmed. PR #471 introduced six lazy values (ctorDefs/methDefs/fieldDefs/eventDefs/propDefs/nestedDefs) in TargetTypeDefinition. These lazies are independent — separate Lazy(T) instances with separate locks — so the F# compiler can force ctorDefs and methDefs concurrently from two threads. Under that concurrent forcing, several underlying caches that pre-date the lazy-caching PR are exposed to concurrent access for the first time:

Cache	Race
ILMethodDefs.getmap()	Thread A assigns lmap = Dictionary() then fills it; Thread B sees non-null lmap and reads it while A is writing → InvalidOperationException
ILTypeDefs.getmap(), ILExportedTypesAndForwarders.getmap()	Same unsynchronised mutable null pattern
mkCacheInt32 / mkCacheGeneric (8 binary-reader caches per assembly)	Same pattern; seekReadTypeRef → cacheTypeRef race
TxTable(T).Get	Plain Dictionary(int,T) written without any lock; concurrent txILTypeRef calls from shared cached MethodInfo/ConstructorInfo objects collide

Fix: I've opened a PR with the following changes:

• ILMethodDefs / ILTypeDefs / ILExportedTypesAndForwarders: build lmap inside lock syncObj so the map is fully populated before any reader can observe it.
• mkCacheInt32 / mkCacheGeneric: each cache now protects every lookup+write with its own lock.
• TxTable(T): switched to ConcurrentDictionary(int, Lazy(T)) so concurrent GetOrAdd calls are safe and the Lazy(T) wrapper guarantees the factory runs at most once per token (preserving TxTable's identity guarantee).

A new regression test (8 threads × 50 iterations, all six GetXxx methods on the same TargetTypeDefinition concurrently) passes cleanly. All 118 tests pass on net8.0.

Generated by Repo Assist for issue #481 · ◷

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@d1d884596e62351dd652ae78465885dd32f0dd7d

[github-actions]

Pull request created: #483

Generated by Repo Assist

[github-actions]

🤖 This is an automated response from Repo Assist.

Thanks for the report, @sergey-tihon! I've investigated the root cause and submitted a fix.

Root Cause

PR #471 introduced lazy caches in TargetTypeDefinition so wrapper objects (ConstructorInfo, MethodInfo, etc.) are shared across callers. This exposed three independent race conditions in the downstream data structures, because those same shared wrapper objects are now accessed from multiple F# compiler threads simultaneously:

Location	Race	Symptom
ILMethodDefs.getmap() / ILTypeDefs.getmap() / ILExportedTypesAndForwarders.getmap()	Thread A sets lmap <- Dictionary() and starts filling it; Thread B sees non-null lmap and reads it while A is inserting	InvalidOperationException: Operations that change non-concurrent collections must have exclusive access
mkCacheInt32 / mkCacheGeneric (binary-reader caches)	Same ref null + unsynchronised Dictionary pattern; two threads race to initialise, one overwrites the other	Stale/empty cache or partially-filled state
TxTable(T).Get	Plain Dictionary(int,'T2) with no locking; concurrent txILType calls on the same wrapper race on writes	NullReferenceException (Windows) or InvalidOperationException

Before #471 each GetConstructors() call created fresh wrapper objects — different threads had separate objects so their TxTable.Get writes were temporally isolated. After #471 the SAME objects are returned to all callers.

Fix

I've submitted a draft PR: #480 (branch repo-assist/fix-thread-safety-lazy-caches-481)

• ILMethodDefs/ILTypeDefs/ILExportedTypesAndForwarders.getmap(): added per-instance syncObj; builds map into local value under lock syncObj, then assigns lmap <- m once fully constructed
• mkCacheInt32/mkCacheGeneric: replaced ref null pattern with Dictionary + syncObj; all accesses under lock
• TxTable(T).Get: replaced Dictionary(int,'T2) with ConcurrentDictionary(int, Lazy<'T2)>; GetOrAdd is race-safe and Lazy (default ExecutionAndPublication) ensures the factory runs exactly once per token

Added a regression test: 8 threads × 50 iterations, all six GetXxx methods concurrently. All 118 tests pass (117 pre-existing + 1 new).

Generated by Repo Assist for issue #481 · ◷

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@d1d884596e62351dd652ae78465885dd32f0dd7d