Fix concurrent type translation minting duplicate target types (#521)#522
Open
Thorium wants to merge 2 commits into
Open
Fix concurrent type translation minting duplicate target types (#521)#522Thorium wants to merge 2 commits into
Thorium wants to merge 2 commits into
Conversation
…jects#521) The F# compiler may drive a type provider from multiple threads (ParallelCompilation is on by default in recent .NET SDKs). Two unsynchronized code paths in ProvidedTypes.fs then race: 1. ProvidedTypesContext type translation (the fix that matters): convProvidedTypeDefToTgt performs check-then-create on the plain Dictionary typeTableFwd. Two concurrent conversions of the same source ProvidedTypeDefinition both miss the lookup and each mint a distinct target ProvidedTypeDefinition, so the compiler sees two unrelated erased types and fails intermittently with FS0193/FS0001 "type X is not compatible with type X". The existing Debug.Assert(not (typeTableFwd.ContainsKey(x))) documents exactly the invariant the race violates. The lookup+create+store is now one atomic step under a re-entrant lock shared by convTypeRef/convType, whose unlocked table reads could otherwise race the locked write. 2. ProvidedTypeDefinition member realization (hardening): evalMembers/ evalInterfaces/evalMethodOverrides drain their delayed queues and append to plain ResizeArrays with no synchronization, so concurrent GetMembers calls can run delayed factories twice and corrupt the backing lists. Realization now runs under a per-instance re-entrant lock, which also guards the bindings lookup cache.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A possible solution, fixes #521
Problem
The F# compiler may drive a type provider from multiple threads —
ParallelCompilationis on by default in recent .NET SDKs — and two code paths inProvidedTypes.fsare not thread-safe under that.1. Cross-targeting type translation (the cause of the intermittent
FS0193/FS0001failures).convProvidedTypeDefToTgtdoes an unsynchronized check-then-create over the plainDictionarytypeTableFwd:Two concurrent conversions of the same source
ProvidedTypeDefinitionboth miss the lookup and each mint a distinct target type. The compiler then holds two reference-distinct copies of the same erased provided type and fails intermittently withFS0193: The type 'X' is not compatible with the type 'X'(identical names on both sides). Provider-side caching cannot prevent this — it deduplicates the source type, while the duplication happens here on the target side.2. Member realization (hardening).
evalMembers/evalInterfaces/evalMethodOverridesdrain their delayed queues and append to plainResizeArrays without synchronization, so concurrentGetMemberscalls can run the delayed factories twice and corrupt the backing lists. Thebindingslookup cache insavehas the same problem.Fix
typeTablesLock) makes lookup+create+store inconvProvidedTypeDefToTgtone atomic step, shared withconvTypeRef/convTypewhose unlockedDictionaryreads could otherwise race the locked write.realizationLock) around member/interface/override realization and the bindings cache.Both use the rename-and-wrap pattern (
xUnsafe+ locked wrapper) to keep the diff reviewable.Lock ordering is acyclic: translation acquires source-type realization locks (via the member cursors); target-type realization acquires the translation lock (via
convMemberDefToTgtin the fresh-member closures); translation never forces target member realization (PatchDeclaringTypeOfMemberonly repoints the declaring type). No deadlock observed across repeated full builds of a heavy consumer.Validation
SQLProvider's test suite (17
SqlDataProvider<...>instantiations of identical static arguments, .NET SDK 10, clean-rebuild loop, single target framework):