Skip to content

Commit fd48659

Browse files
michaelstaibmichaelstaib
authored
Fixed issue with global context data being disposed to early. (#7593)
1 parent 07eb1aa commit fd48659

File tree

6 files changed

+138
-31
lines changed

6 files changed

+138
-31
lines changed

‎src/HotChocolate/Core/src/Execution/Pipeline/OperationExecutionMiddleware.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ private async Task ExecuteOperationRequestAsync(
9090
if (operation.Definition.Operation is OperationType.Subscription)
9191
{
9292
// since the request context is pooled we need to clone the context for
93-
// long running executions.
93+
// long-running executions.
9494
var cloned = context.Clone();
9595

9696
context.Result = await _subscriptionExecutor

‎src/HotChocolate/Core/src/Execution/RequestExecutor.cs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ internal async Task<IExecutionResult> ExecuteAsync(
103103
services.InitializeDataLoaderScope();
104104
}
105105

106-
var context = _contextPool.Get();
106+
RequestContext? context = _contextPool.Get();
107107

108108
try
109109
{
@@ -128,15 +128,23 @@ internal async Task<IExecutionResult> ExecuteAsync(
128128

129129
if (context.Result.IsStreamResult())
130130
{
131+
var localContext = context;
131132
context.Result.RegisterForCleanup(scope);
133+
context.Result.RegisterForCleanup(() => _contextPool.Return(localContext));
132134
scope = null;
135+
context = null;
136+
return localContext.Result;
133137
}
134138

135139
return context.Result;
136140
}
137141
finally
138142
{
139-
_contextPool.Return(context);
143+
if (context is not null)
144+
{
145+
_contextPool.Return(context);
146+
}
147+
140148
scope?.Dispose();
141149
}
142150
}
@@ -297,7 +305,7 @@ private static async Task UnwrapBatchItemResultAsync(
297305

298306
private void EnrichContext(IRequestContext context)
299307
{
300-
if (_enricher.Length <= 0)
308+
if (_enricher.Length == 0)
301309
{
302310
return;
303311
}

‎src/HotChocolate/Core/test/Execution.Tests/DeferAndStreamTestSchema.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,12 @@ public async Task<bool> Wait(int m, CancellationToken ct)
6161
await Task.Delay(m, ct);
6262
return true;
6363
}
64+
65+
public async Task<Stateful> EnsureState()
66+
{
67+
await Task.Delay(150);
68+
return new Stateful();
69+
}
6470
}
6571

6672
public class Person
@@ -81,4 +87,13 @@ public async Task<string> GetNameAsync(CancellationToken cancellationToken)
8187
return _name;
8288
}
8389
}
90+
91+
public class Stateful
92+
{
93+
public async Task<string> GetState([GlobalState] string requestState)
94+
{
95+
await Task.Delay(250);
96+
return requestState;
97+
}
98+
}
8499
}

‎src/HotChocolate/Core/test/Execution.Tests/DeferTests.cs

Lines changed: 97 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,15 @@ public async Task InlineFragment_Defer()
1212

1313
// act
1414
var result = await executor.ExecuteAsync(
15-
@"{
15+
"""
16+
{
1617
... @defer {
17-
person(id: ""UGVyc29uOjE="") {
18+
person(id: "UGVyc29uOjE=") {
1819
id
1920
}
2021
}
21-
}");
22+
}
23+
""");
2224

2325
Assert.IsType<ResponseStream>(result).MatchSnapshot();
2426
}
@@ -31,16 +33,18 @@ public async Task InlineFragment_Defer_Nested()
3133

3234
// act
3335
var result = await executor.ExecuteAsync(
34-
@"{
36+
"""
37+
{
3538
... @defer {
36-
person(id: ""UGVyc29uOjE="") {
39+
person(id: "UGVyc29uOjE=") {
3740
id
3841
... @defer {
3942
name
4043
}
4144
}
4245
}
43-
}");
46+
}
47+
""");
4448

4549
Assert.IsType<ResponseStream>(result).MatchSnapshot();
4650
}
@@ -72,13 +76,15 @@ public async Task InlineFragment_Defer_If_Set_To_false()
7276

7377
// act
7478
var result = await executor.ExecuteAsync(
75-
@"{
79+
"""
80+
{
7681
... @defer(if: false) {
77-
person(id: ""UGVyc29uOjE="") {
82+
person(id: "UGVyc29uOjE=") {
7883
id
7984
}
8085
}
81-
}");
86+
}
87+
""");
8288

8389
Assert.IsType<OperationResult>(result).MatchSnapshot();
8490
}
@@ -94,14 +100,16 @@ public async Task InlineFragment_Defer_If_Variable_Set_To_false()
94100
OperationRequestBuilder
95101
.New()
96102
.SetDocument(
97-
@"query($defer: Boolean!) {
103+
"""
104+
query($defer: Boolean!) {
98105
... @defer(if: $defer) {
99-
person(id: ""UGVyc29uOjE="") {
106+
person(id: "UGVyc29uOjE=") {
100107
id
101108
}
102109
}
103-
}")
104-
.SetVariableValues(new Dictionary<string, object?> { {"defer", false }, })
110+
}
111+
""")
112+
.SetVariableValues(new Dictionary<string, object?> { { "defer", false }, })
105113
.Build());
106114

107115
Assert.IsType<OperationResult>(result).MatchSnapshot();
@@ -115,15 +123,17 @@ public async Task FragmentSpread_Defer()
115123

116124
// act
117125
var result = await executor.ExecuteAsync(
118-
@"{
126+
"""
127+
{
119128
... Foo @defer
120129
}
121130
122131
fragment Foo on Query {
123-
person(id: ""UGVyc29uOjE="") {
132+
person(id: "UGVyc29uOjE=") {
124133
id
125134
}
126-
}");
135+
}
136+
""");
127137

128138
Assert.IsType<ResponseStream>(result).MatchSnapshot();
129139
}
@@ -162,15 +172,17 @@ public async Task FragmentSpread_Defer_Label_Set_To_abc()
162172

163173
// act
164174
var result = await executor.ExecuteAsync(
165-
@"{
166-
... Foo @defer(label: ""abc"")
175+
"""
176+
{
177+
... Foo @defer(label: "abc")
167178
}
168179
169180
fragment Foo on Query {
170-
person(id: ""UGVyc29uOjE="") {
181+
person(id: "UGVyc29uOjE=") {
171182
id
172183
}
173-
}");
184+
}
185+
""");
174186

175187
Assert.IsType<ResponseStream>(result).MatchSnapshot();
176188
}
@@ -183,15 +195,17 @@ public async Task FragmentSpread_Defer_If_Set_To_false()
183195

184196
// act
185197
var result = await executor.ExecuteAsync(
186-
@"{
198+
"""
199+
{
187200
... Foo @defer(if: false)
188201
}
189202
190203
fragment Foo on Query {
191-
person(id: ""UGVyc29uOjE="") {
204+
person(id: "UGVyc29uOjE=") {
192205
id
193206
}
194-
}");
207+
}
208+
""");
195209

196210
Assert.IsType<OperationResult>(result).MatchSnapshot();
197211
}
@@ -207,18 +221,74 @@ public async Task FragmentSpread_Defer_If_Variable_Set_To_false()
207221
OperationRequestBuilder
208222
.New()
209223
.SetDocument(
210-
@"query ($defer: Boolean!) {
224+
"""
225+
query ($defer: Boolean!) {
211226
... Foo @defer(if: $defer)
212227
}
213228
214229
fragment Foo on Query {
215-
person(id: ""UGVyc29uOjE="") {
230+
person(id: "UGVyc29uOjE=") {
216231
id
217232
}
218-
}")
219-
.SetVariableValues(new Dictionary<string, object?> { {"defer", false }, })
233+
}
234+
""")
235+
.SetVariableValues(new Dictionary<string, object?> { { "defer", false }, })
220236
.Build());
221237

222238
Assert.IsType<OperationResult>(result).MatchSnapshot();
223239
}
240+
241+
[Fact]
242+
public async Task Ensure_GlobalState_Is_Passed_To_DeferContext_Stacked_Defer()
243+
{
244+
// arrange
245+
var executor = await DeferAndStreamTestSchema.CreateAsync();
246+
247+
// act
248+
var result = await executor.ExecuteAsync(
249+
OperationRequestBuilder
250+
.New()
251+
.SetDocument(
252+
"""
253+
{
254+
... @defer {
255+
ensureState {
256+
... @defer {
257+
state
258+
}
259+
}
260+
}
261+
}
262+
""")
263+
.SetGlobalState("requestState", "state 123")
264+
.Build());
265+
266+
Assert.IsType<ResponseStream>(result).MatchSnapshot();
267+
}
268+
269+
[Fact]
270+
public async Task Ensure_GlobalState_Is_Passed_To_DeferContext_Single_Defer()
271+
{
272+
// arrange
273+
var executor = await DeferAndStreamTestSchema.CreateAsync();
274+
275+
// act
276+
var result = await executor.ExecuteAsync(
277+
OperationRequestBuilder
278+
.New()
279+
.SetDocument(
280+
"""
281+
{
282+
ensureState {
283+
... @defer {
284+
state
285+
}
286+
}
287+
}
288+
""")
289+
.SetGlobalState("requestState", "state 123")
290+
.Build());
291+
292+
Assert.IsType<ResponseStream>(result).MatchSnapshot();
293+
}
224294
}

‎src/HotChocolate/Core/test/Execution.Tests/__snapshots__/DeferTests.Ensure_GlobalState_Is_Passed_To_DeferContext_Single_Defer.snap

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"data": {
3+
"ensureState": {
4+
"state": "state 123"
5+
}
6+
}
7+
}

‎src/HotChocolate/Core/test/Execution.Tests/__snapshots__/DeferTests.Ensure_GlobalState_Is_Passed_To_DeferContext_Stacked_Defer.snap

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"data": {
3+
"ensureState": {
4+
"state": "state 123"
5+
}
6+
}
7+
}

0 commit comments

Comments
 (0)