From cfb817aebb8d50c618740bb2cda37167e9d959a6 Mon Sep 17 00:00:00 2001 From: Jeremy Sinner Date: Wed, 24 Jun 2026 15:31:44 +0200 Subject: [PATCH 1/2] fix: removes two entry points for memory leaks Makes Token.prev a weak var preventing retention if a cyclec occur in the graph. ValidationContext no longer captures itself strongly in assigning onError; uses an override of report instead. Regression tests were added to validate the fixes --- Sources/GraphQL/Language/AST.swift | 2 +- .../Validation/ValidationContext.swift | 7 +- .../GraphQLMemoryRetentionTests.swift | 65 +++++++++++++++++++ 3 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 Tests/GraphQLTests/GraphQLMemoryRetentionTests.swift diff --git a/Sources/GraphQL/Language/AST.swift b/Sources/GraphQL/Language/AST.swift index 1b6e4e58..828c53a6 100644 --- a/Sources/GraphQL/Language/AST.swift +++ b/Sources/GraphQL/Language/AST.swift @@ -91,7 +91,7 @@ public final class Token: @unchecked Sendable { * including ignored tokens. is always the first node and * the last. */ - public let prev: Token? + public private(set) weak var prev: Token? public internal(set) var next: Token? init( diff --git a/Sources/GraphQL/Validation/ValidationContext.swift b/Sources/GraphQL/Validation/ValidationContext.swift index 94f95fa1..a9923eb2 100644 --- a/Sources/GraphQL/Validation/ValidationContext.swift +++ b/Sources/GraphQL/Validation/ValidationContext.swift @@ -176,9 +176,10 @@ public final class ValidationContext: ASTValidationContext { recursiveVariableUsages = [:] super.init(ast: ast) { _ in } - onError = { error in - self.errors.append(error) - } + } + + public override func report(error: GraphQLError) { + errors.append(error) } func getSchema() -> GraphQLSchema? { diff --git a/Tests/GraphQLTests/GraphQLMemoryRetentionTests.swift b/Tests/GraphQLTests/GraphQLMemoryRetentionTests.swift new file mode 100644 index 00000000..4d460331 --- /dev/null +++ b/Tests/GraphQLTests/GraphQLMemoryRetentionTests.swift @@ -0,0 +1,65 @@ +import Testing + +@testable import GraphQL + +// Holds a weak reference to a Token so we can observe when ARC frees it. +private final class WeakTokenBox { + weak var token: Token? +} + +@Suite struct GraphQLMemoryRetentionTests { + private func makeSchema() throws -> GraphQLSchema { + try GraphQLSchema( + query: GraphQLObjectType( + name: "Query", + fields: ["hello": GraphQLField(type: GraphQLString)] + ) + ) + } + + // Regression test for Token.prev being a strong reference. + // + // Adjacent tokens in the linked list formed a mutual retain cycle: + // SOF.next → T1 and T1.prev → SOF. When the Document went out of scope, + // neither token could be freed. Making prev `weak` breaks the cycle. + @Test func tokenChainIsReleasedAfterDocumentGoesOutOfScope() throws { + let box = WeakTokenBox() + + func parseAndCapture() throws { + let document = try parse(source: "{ hello }") + // startToken is SOF (no prev). Capture the next token, which + // has a .prev back to SOF — the exact edge the fix targets. + box.token = document.loc?.startToken.next + #expect(box.token != nil) + } + + try parseAndCapture() + #expect(box.token == nil) + } + + // Regression test for ValidationContext.init capturing `self` in a closure. + // + // The onError closure stored on ASTValidationContext captured ValidationContext + // strongly, creating a cycle that prevented the context (and the Document it + // holds) from ever being freed. Overriding report(error:) instead eliminates + // the closure and the cycle. + // + // SOF (startToken) has no .prev, so it has no token-chain cycle. If it is + // retained after this scope it must be because ValidationContext leaked and + // is still holding a copy of the Document. + @Test func validationContextReleasesDocumentAfterValidation() throws { + let schema = try makeSchema() + let box = WeakTokenBox() + + func validateAndCapture() throws { + let document = try parse(source: "{ hello }") + box.token = document.loc?.startToken // SOF — no prev, no token cycle + let errors = validate(schema: schema, ast: document) + #expect(errors.isEmpty) + #expect(box.token != nil) + } + + try validateAndCapture() + #expect(box.token == nil) + } +} From 9492055ab1256aa8ee9661237d1aea5f0dc172fd Mon Sep 17 00:00:00 2001 From: Jeremy Sinner Date: Thu, 25 Jun 2026 15:57:28 +0200 Subject: [PATCH 2/2] fix: remove errors array, add onError closure to ValidationContext --- Sources/GraphQL/Validation/Validate.swift | 5 +++-- Sources/GraphQL/Validation/ValidationContext.swift | 10 ++-------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/Sources/GraphQL/Validation/Validate.swift b/Sources/GraphQL/Validation/Validate.swift index 2073f9cd..92d5d09e 100644 --- a/Sources/GraphQL/Validation/Validate.swift +++ b/Sources/GraphQL/Validation/Validate.swift @@ -50,14 +50,15 @@ func visit( typeInfo: TypeInfo, documentAST: Document ) -> [GraphQLError] { - let context = ValidationContext(schema: schema, ast: documentAST, typeInfo: typeInfo) + var errors = [GraphQLError]() + let context = ValidationContext(schema: schema, ast: documentAST, typeInfo: typeInfo, onError: { errors.append($0) }) let visitors = rules.map { rule in rule(context) } // Visit the whole document with each instance of all provided rules. visit( root: documentAST, visitor: visitWithTypeInfo(typeInfo: typeInfo, visitor: visitInParallel(visitors: visitors)) ) - return context.errors + return errors } /// Utility function which asserts a SDL document is valid by throwing an error diff --git a/Sources/GraphQL/Validation/ValidationContext.swift b/Sources/GraphQL/Validation/ValidationContext.swift index a9923eb2..01874912 100644 --- a/Sources/GraphQL/Validation/ValidationContext.swift +++ b/Sources/GraphQL/Validation/ValidationContext.swift @@ -164,22 +164,16 @@ public typealias SDLValidationRule = @Sendable (SDLValidationContext) -> Visitor public final class ValidationContext: ASTValidationContext { public let schema: GraphQLSchema let typeInfo: TypeInfo - var errors: [GraphQLError] var variableUsages: [HasSelectionSet: [VariableUsage]] var recursiveVariableUsages: [OperationDefinition: [VariableUsage]] - init(schema: GraphQLSchema, ast: Document, typeInfo: TypeInfo) { + init(schema: GraphQLSchema, ast: Document, typeInfo: TypeInfo, onError: @escaping (GraphQLError) -> Void) { self.schema = schema self.typeInfo = typeInfo - errors = [] variableUsages = [:] recursiveVariableUsages = [:] - super.init(ast: ast) { _ in } - } - - public override func report(error: GraphQLError) { - errors.append(error) + super.init(ast: ast, onError: onError) } func getSchema() -> GraphQLSchema? {