Skip to content

Commit d3366a2

Browse files
Merge pull request #182 from jwsinner/fix/memory-leak
Removes two entry points for memory leaks
2 parents 17c68cf + 9492055 commit d3366a2

4 files changed

Lines changed: 71 additions & 10 deletions

File tree

Sources/GraphQL/Language/AST.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public final class Token: @unchecked Sendable {
9191
* including ignored tokens. <SOF> is always the first node and <EOF>
9292
* the last.
9393
*/
94-
public let prev: Token?
94+
public private(set) weak var prev: Token?
9595
public internal(set) var next: Token?
9696

9797
init(

Sources/GraphQL/Validation/Validate.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,15 @@ func visit(
5050
typeInfo: TypeInfo,
5151
documentAST: Document
5252
) -> [GraphQLError] {
53-
let context = ValidationContext(schema: schema, ast: documentAST, typeInfo: typeInfo)
53+
var errors = [GraphQLError]()
54+
let context = ValidationContext(schema: schema, ast: documentAST, typeInfo: typeInfo, onError: { errors.append($0) })
5455
let visitors = rules.map { rule in rule(context) }
5556
// Visit the whole document with each instance of all provided rules.
5657
visit(
5758
root: documentAST,
5859
visitor: visitWithTypeInfo(typeInfo: typeInfo, visitor: visitInParallel(visitors: visitors))
5960
)
60-
return context.errors
61+
return errors
6162
}
6263

6364
/// Utility function which asserts a SDL document is valid by throwing an error

Sources/GraphQL/Validation/ValidationContext.swift

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -164,21 +164,16 @@ public typealias SDLValidationRule = @Sendable (SDLValidationContext) -> Visitor
164164
public final class ValidationContext: ASTValidationContext {
165165
public let schema: GraphQLSchema
166166
let typeInfo: TypeInfo
167-
var errors: [GraphQLError]
168167
var variableUsages: [HasSelectionSet: [VariableUsage]]
169168
var recursiveVariableUsages: [OperationDefinition: [VariableUsage]]
170169

171-
init(schema: GraphQLSchema, ast: Document, typeInfo: TypeInfo) {
170+
init(schema: GraphQLSchema, ast: Document, typeInfo: TypeInfo, onError: @escaping (GraphQLError) -> Void) {
172171
self.schema = schema
173172
self.typeInfo = typeInfo
174-
errors = []
175173
variableUsages = [:]
176174
recursiveVariableUsages = [:]
177175

178-
super.init(ast: ast) { _ in }
179-
onError = { error in
180-
self.errors.append(error)
181-
}
176+
super.init(ast: ast, onError: onError)
182177
}
183178

184179
func getSchema() -> GraphQLSchema? {
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import Testing
2+
3+
@testable import GraphQL
4+
5+
// Holds a weak reference to a Token so we can observe when ARC frees it.
6+
private final class WeakTokenBox {
7+
weak var token: Token?
8+
}
9+
10+
@Suite struct GraphQLMemoryRetentionTests {
11+
private func makeSchema() throws -> GraphQLSchema {
12+
try GraphQLSchema(
13+
query: GraphQLObjectType(
14+
name: "Query",
15+
fields: ["hello": GraphQLField(type: GraphQLString)]
16+
)
17+
)
18+
}
19+
20+
// Regression test for Token.prev being a strong reference.
21+
//
22+
// Adjacent tokens in the linked list formed a mutual retain cycle:
23+
// SOF.next → T1 and T1.prev → SOF. When the Document went out of scope,
24+
// neither token could be freed. Making prev `weak` breaks the cycle.
25+
@Test func tokenChainIsReleasedAfterDocumentGoesOutOfScope() throws {
26+
let box = WeakTokenBox()
27+
28+
func parseAndCapture() throws {
29+
let document = try parse(source: "{ hello }")
30+
// startToken is SOF (no prev). Capture the next token, which
31+
// has a .prev back to SOF — the exact edge the fix targets.
32+
box.token = document.loc?.startToken.next
33+
#expect(box.token != nil)
34+
}
35+
36+
try parseAndCapture()
37+
#expect(box.token == nil)
38+
}
39+
40+
// Regression test for ValidationContext.init capturing `self` in a closure.
41+
//
42+
// The onError closure stored on ASTValidationContext captured ValidationContext
43+
// strongly, creating a cycle that prevented the context (and the Document it
44+
// holds) from ever being freed. Overriding report(error:) instead eliminates
45+
// the closure and the cycle.
46+
//
47+
// SOF (startToken) has no .prev, so it has no token-chain cycle. If it is
48+
// retained after this scope it must be because ValidationContext leaked and
49+
// is still holding a copy of the Document.
50+
@Test func validationContextReleasesDocumentAfterValidation() throws {
51+
let schema = try makeSchema()
52+
let box = WeakTokenBox()
53+
54+
func validateAndCapture() throws {
55+
let document = try parse(source: "{ hello }")
56+
box.token = document.loc?.startToken // SOF — no prev, no token cycle
57+
let errors = validate(schema: schema, ast: document)
58+
#expect(errors.isEmpty)
59+
#expect(box.token != nil)
60+
}
61+
62+
try validateAndCapture()
63+
#expect(box.token == nil)
64+
}
65+
}

0 commit comments

Comments
 (0)