Skip to content

Fix thread-safety bug in QueryGenerator caused by static mutable field#116

Open
penspanic wants to merge 1 commit intogenaray:masterfrom
penspanic:fix/query-generator-thread-safety
Open

Fix thread-safety bug in QueryGenerator caused by static mutable field#116
penspanic wants to merge 1 commit intogenaray:masterfrom
penspanic:fix/query-generator-thread-safety

Conversation

@penspanic
Copy link
Copy Markdown

Problem

QueryGenerator uses a static Dictionary<ISymbol, List<IMethodSymbol>> _classToMethods field that is shared across all concurrent generator invocations. During parallel builds (e.g. multiple projects referencing the generator, or multi-targeting), this causes intermittent:

Generator 'QueryGenerator' failed to generate source. It will not contribute to the output
and compilation errors may occur as a result. Exception was of type 'InvalidOperationException'
with message 'Collection was modified; enumeration operation may not execute.'

The error occurs because one thread enumerates _classToMethods in the foreach loop while another thread reassigns or mutates it in a concurrent Generate() call.

Fix

Remove the static field and replace it with a local variable inside Generate(), passed as a parameter to AddMethodToClass(). Since the dictionary is already re-created at the start of every Generate() call, there is no need for it to persist across calls.

Test

Added ConcurrencyTest that runs 8 parallel generator invocations with [Repeat(10)]. Before the fix, this fails 100% of the time. After the fix, it passes 100%.

The _classToMethods dictionary was declared as a static field, causing
"Collection was modified; enumeration operation may not execute" errors
during parallel builds when multiple compilations invoke the generator
concurrently.

Replace the static field with a local variable in Generate() and pass
it as a parameter to AddMethodToClass(). Add a concurrency test that
reproduces the race condition with parallel generator invocations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant