Run feature factories outside the OperationFeatureCollection write lock

[apereiratl]

GetOrSetSafe invoked the caller's factory while holding _writeLock, which lets two execution locks be taken in opposite orders:

Operation.GetSelectionSet takes the operation lock, then CompileSelectionSet writes selection features (OperationCompiler sets the optimizer feature), so it holds the operation lock and wants the feature lock.

GetOrCreateSelectorExpression takes the feature lock, then the factory builds the selector expression, which calls Operation.GetSelectionSet, so it holds the feature lock and wants the operation lock.

Two threads hitting both paths on one operation deadlock permanently. Every later request completing a composite value then blocks on the same operation lock, the pool injects threads to replace them, and the server stops answering. Only the first execution of an operation compiles selection sets and selector expressions, so it takes a schema with many distinct operations to hit reliably, but it needs no unusual input and does not recover.

The factory now runs before the lock is taken, which is entered only to store the result. Racing callers can each build a value, the first stored wins, and every caller receives that instance, matching ConcurrentDictionary.GetOrAdd.

[Copilot]

Pull request overview

This pull request fixes a deadlock risk in OperationFeatureCollection by ensuring GetOrSetSafe executes feature factories outside the collection’s _writeLock, only taking the lock to store the computed value (double-checking for an existing value first). This aligns the behavior with ConcurrentDictionary.GetOrAdd semantics under contention.

Changes:

• Run GetOrSetSafe factories outside _writeLock for both operation-level and selection-level feature storage, then lock only to publish the winning instance.
• Add concurrency-focused tests to ensure factories aren’t invoked while holding the write lock and that racing callers observe the same stored instance.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/HotChocolate/Core/src/Types/Execution/Processing/OperationFeatureCollection.cs	Updates operation-level GetOrSetSafe to invoke factories outside _writeLock and publish the first-created value.
src/HotChocolate/Core/src/Types/Execution/Processing/OperationFeatureCollection.Selections.cs	Applies the same “factory outside lock” pattern for selection-scoped features.
src/HotChocolate/Core/test/Execution.Tests/Processing/OperationFeatureCollectionTests.cs	Adds tests intended to validate no lock is held during factory execution and that racing callers return the same stored instance.

Suppressed comments (1)

src/HotChocolate/Core/test/Execution.Tests/Processing/OperationFeatureCollectionTests.cs:65

• Same issue as the selection-factory test: the waits inside the factory don’t assert success. If the writer only completes after the factory returns (because the write lock was held), the event may still be set before the outer assertions, letting the test pass even though the lock was held during factory execution.

            concurrentWriteFinished.Wait(s_timeout);
            writer.Wait(s_timeout);
            return new Feature();

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.