Repository navigation
Fix deferred groups below a nulled object still running and blocking later group members - #184
Open
michaelstaib wants to merge 1 commit into
Conversation
…ed before the group runs
Copilot stopped reviewing on behalf of
michaelstaib due to an error
November 5, 2026 14:15
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Copilot review overview
2 open issues
What changed in this PR
Adds safeguards to deferred execution when a deferred branch’s parent is nulled, ensuring later execution-group members still run and no deferred payload is announced.
Changes:
- Add/extend tests covering
@deferbehavior when ancestors are nulled before/after deferred groups run. - Track the
ResultDocumentfor deferred branches and discard branches whose target object becomes null before delivery. - Introduce execution-group slot releasing for skipped/dead deferred groups so subsequent members can proceed.
| File | Description |
|---|---|
| src/HotChocolate/Core/test/Execution.Tests/ExecutionGroups/ExecutionGroupTests.cs | Adds a regression test ensuring “later members” still execute when a deferred member is under a nulled object. |
| src/HotChocolate/Core/test/Execution.Tests/DeferBelowNulledParentTests.cs | New focused tests for discarding/avoiding deferred group announcement when ancestors null out. |
| src/HotChocolate/Core/src/Types/Text/Json/ResultElement.cs | Exposes the owning ResultDocument needed to determine nulling state for deferred branches. |
| src/HotChocolate/Core/src/Types/Execution/Processing/Tasks/ResolverTaskFactory.cs | Passes ResultDocument into branch registration so null-detection can work. |
| src/HotChocolate/Core/src/Types/Execution/Processing/Tasks/DeferTask.cs | Discards deferred branches that will never be delivered and releases group slots so later tasks can start. |
| src/HotChocolate/Core/src/Types/Execution/Processing/ExecutionGroups/ExecutionGroupScope.cs | Adds an API to release a selection’s slot plus descendants (used when skipping deferred group members). |
| src/HotChocolate/Core/src/Types/Execution/Processing/DeferExecutionCoordinator.cs | Extends branch registration with document tracking and adds TryDiscardNulledBranch for early discard. |
Review effort: Lite
Comment on lines
+39
to
+40
| internal ResultDocument Document => _parent; | ||
|
|
Comment on lines
+89
to
+102
| /// <summary> | ||
| /// Unregisters the deferred branch <paramref name="branchId"/> if its object was nulled before its parent | ||
| /// was delivered, returning <c>true</c> if the branch will never be delivered. | ||
| /// </summary> | ||
| public bool TryDiscardNulledBranch(int branchId) | ||
| { | ||
| AssertInitialized(); | ||
|
|
||
| lock (_sync) | ||
| { | ||
| if (!_branchLookup.TryGetValue(branchId, out var branch)) | ||
| { | ||
| return true; | ||
| } |
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 deferred group whose object was nulled before the group ran was still resolved. Its announcement was already suppressed, but its resolvers ran anyway. When such a group held an execution group member, that member never ended its turn, so a later member of the same execution group waited forever.
Changes:
DeferTaskchecks whether its branch sits below a nulled object, using the same path resolution as the announce check, and skips the group if it does. A branch that was already announced is never skipped.ExecutionGroupScope.ReleaseSlotsreleases the slots of its execution group members, so later members run.Behavior change: a deferred group whose object was nulled before it runs is no longer resolved, and its execution group members are skipped.
Stacked on #175.
Tests:
DeferBelowNulledParentTestscover a nullable ancestor field nulled before the group runs, a parent nulled before the group runs, and a parent nulled after the group resolved.ExecutionGroupTestscover a later member running when a deferred member is below a nulled object, which hung before the fix. Execution.Tests pass on net10.0, and the touched classes pass on net8.0.