From 9b7b04cafd78dd83e3de325c74b342f2a130dfe6 Mon Sep 17 00:00:00 2001 From: Abdullah Alaqeel Date: Sat, 11 Jul 2026 17:13:51 +0300 Subject: [PATCH 1/2] fix: iterate instead of recurse in external dereferencing depth loop Components.externallyDereference implemented its depth/iteration loop recursively, nesting one large async frame per pass. For .full on a document with chained external $refs, that stacked enough multi-KB frames to overflow nightly-focal's cooperative-thread stack (intermittent SIGSEGV in test_example). It was also a latent scalability bug for deep external-ref chains. Convert the recursive self-call to a while loop by extracting one pass into a performExternalDereferencePass helper. Per-pass stack cost goes from O(passes) recursive frames to O(1). Behavior-identical; non-breaking. Applied to both OpenAPIKit and OpenAPIKit30. Fixes #505. --- .../Components Object/Components.swift | 32 +++++++++++++------ .../Components Object/Components.swift | 32 +++++++++++++------ 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/Sources/OpenAPIKit/Components Object/Components.swift b/Sources/OpenAPIKit/Components Object/Components.swift index 9f555b091..13316529e 100644 --- a/Sources/OpenAPIKit/Components Object/Components.swift +++ b/Sources/OpenAPIKit/Components Object/Components.swift @@ -441,10 +441,27 @@ extension OpenAPI.Components { extension OpenAPI.Components { #if ExternalLoading internal mutating func externallyDereference(with loader: Loader.Type, depth: ExternalDereferenceDepth = .iterations(1), context: [Loader.Message] = []) async throws -> [Loader.Message] { - if case let .iterations(number) = depth, - number <= 0 { - return context + var remainingIterations: Int? = if case .iterations(let number) = depth { number } else { nil } + var accumulatedMessages = context + + while true { + if let remainingIterations, remainingIterations <= 0 { + return accumulatedMessages + } + + let (noNewComponents, newMessages) = try await performExternalDereferencePass(with: loader, context: accumulatedMessages) + accumulatedMessages = newMessages + + if noNewComponents { return accumulatedMessages } + + remainingIterations = remainingIterations.map { $0 - 1 } } + } + + /// One pass of external dereferencing. Snapshot each component map, dereference it, + /// and merge any newly-loaded components into `self`. Returns `(noNewComponents, messages)`, + /// where `noNewComponents` is true when the pass loaded nothing new (i.e. dereferencing is complete). + private mutating func performExternalDereferencePass(with loader: Loader.Type, context: [Loader.Message]) async throws -> (noNewComponents: Bool, messages: [Loader.Message]) { // NOTE: The links and callbacks related code commented out below pushes Swift 5.8 and 5.9 // over the edge and you get exit code 137 crashes in CI. @@ -522,7 +539,7 @@ extension OpenAPI.Components { let newMessages = try await context + m1 + m2 + m3 + m4 + m5 + m6 + m7 + m8 + m9 + m10 + m11 - if noNewComponents { return newMessages } + if noNewComponents { return (true, newMessages) } try merge(c1Resolved) try merge(c2Resolved) @@ -536,12 +553,7 @@ extension OpenAPI.Components { try merge(c10Resolved) try merge(c11Resolved) - switch depth { - case .iterations(let number): - return try await externallyDereference(with: loader, depth: .iterations(number - 1), context: newMessages) - case .full: - return try await externallyDereference(with: loader, depth: .full, context: newMessages) - } + return (false, newMessages) } #endif } diff --git a/Sources/OpenAPIKit30/Components Object/Components.swift b/Sources/OpenAPIKit30/Components Object/Components.swift index 8a65441e4..d204fb3df 100644 --- a/Sources/OpenAPIKit30/Components Object/Components.swift +++ b/Sources/OpenAPIKit30/Components Object/Components.swift @@ -318,10 +318,27 @@ extension OpenAPI.Components { extension OpenAPI.Components { #if ExternalLoading internal mutating func externallyDereference(with loader: Loader.Type, depth: ExternalDereferenceDepth = .iterations(1), context: [Loader.Message] = []) async throws -> [Loader.Message] { - if case let .iterations(number) = depth, - number <= 0 { - return context + var remainingIterations: Int? = if case .iterations(let number) = depth { number } else { nil } + var accumulatedMessages = context + + while true { + if let remainingIterations, remainingIterations <= 0 { + return accumulatedMessages + } + + let (noNewComponents, newMessages) = try await performExternalDereferencePass(with: loader, context: accumulatedMessages) + accumulatedMessages = newMessages + + if noNewComponents { return accumulatedMessages } + + remainingIterations = remainingIterations.map { $0 - 1 } } + } + + /// One pass of external dereferencing. Snapshot each component map, dereference it, + /// and merge any newly-loaded components into `self`. Returns `(noNewComponents, messages)`, + /// where `noNewComponents` is true when the pass loaded nothing new (i.e. dereferencing is complete). + private mutating func performExternalDereferencePass(with loader: Loader.Type, context: [Loader.Message]) async throws -> (noNewComponents: Bool, messages: [Loader.Message]) { // NOTE: The links and callbacks related code commented out below pushes Swift 5.8 and 5.9 // over the edge and you get exit code 137 crashes in CI. @@ -394,7 +411,7 @@ extension OpenAPI.Components { let newMessages = try await context + m1 + m2 + m3 + m4 + m5 + m6 + m7 + m8 + m9 + m10 - if noNewComponents { return newMessages } + if noNewComponents { return (true, newMessages) } try merge(c1Resolved) try merge(c2Resolved) @@ -407,12 +424,7 @@ extension OpenAPI.Components { try merge(c9Resolved) try merge(c10Resolved) - switch depth { - case .iterations(let number): - return try await externallyDereference(with: loader, depth: .iterations(number - 1), context: newMessages) - case .full: - return try await externallyDereference(with: loader, depth: .full, context: newMessages) - } + return (false, newMessages) } #endif } From d48415956d02f751cc242de267338092f95d0269 Mon Sep 17 00:00:00 2001 From: Abdullah Alaqeel Date: Sun, 12 Jul 2026 03:04:57 +0300 Subject: [PATCH 2/2] test: document that cyclic external refs converge under .full MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a test with a cyclic external-ref graph (A→B→A) to prove that externallyDereference(with: .full) converges — the merge-collision (detectCollision keeps the old component) discards re-loaded duplicates and the loop terminates once all refs are internal. --- .../ExternalDereferencingDocumentTests.swift | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/Tests/OpenAPIKitTests/Document/ExternalDereferencingDocumentTests.swift b/Tests/OpenAPIKitTests/Document/ExternalDereferencingDocumentTests.swift index aaced8fc7..001f67e89 100644 --- a/Tests/OpenAPIKitTests/Document/ExternalDereferencingDocumentTests.swift +++ b/Tests/OpenAPIKitTests/Document/ExternalDereferencingDocumentTests.swift @@ -353,5 +353,60 @@ final class ExternalDereferencingDocumentTests: XCTestCase { "file://./schemas/string_param.json#"] ) } + + func test_cyclicExternalReferencesConvergeAtFullDepth() async throws { + // Documents that A→B→A cycle: `.full` must converge, not infinite-loop. + // Merge-collision (detectCollision keeps old) discards re-loaded components. + struct CyclicLoader: ExternalLoader { + typealias Message = String + + static func load(_ url: URL) async throws -> (T, [Message]) where T: Decodable { + let key = try componentKey(type: T.self, at: url) + let files: [String: Data] = [ + "schemas_a_json": """ + {"type":"object","properties":{"next":{"$ref":"file://./schemas/b.json"}}} + """.data(using: .utf8)!, + "schemas_b_json": """ + {"type":"object","properties":{"next":{"$ref":"file://./schemas/a.json"}}} + """.data(using: .utf8)! + ] + let data = try XCTUnwrap(files[key.rawValue]) + let decoded = try JSONDecoder().decode(T.self, from: data) + return (decoded, [url.absoluteString]) + } + + static func componentKey(type: T.Type, at url: URL) throws -> OpenAPI.ComponentKey { + let urlString = url.pathComponents.dropFirst() + .joined(separator: "_") + .replacingOccurrences(of: ".", with: "_") + return try .forceInit(rawValue: urlString) + } + } + + let document = OpenAPI.Document( + info: .init(title: "cyclic test", version: "1.0.0"), + servers: [], + paths: [:], + components: .init( + schemas: ["entry": .reference(.external(URL(string: "file://./schemas/a.json")!))] + ) + ) + + var docCopy = document + _ = try await docCopy.externallyDereference(with: CyclicLoader.self, depth: .full) + + // .full converged despite the A→B→A cycle. + let aSchema = try XCTUnwrap(docCopy.components.schemas["schemas_a_json"]) + let bSchema = try XCTUnwrap(docCopy.components.schemas["schemas_b_json"]) + + // Both schemas' external refs resolved to internal references. + guard case .object(_, let aCtx) = aSchema.value, + case .object(_, let bCtx) = bSchema.value else { + XCTFail("expected both schemas to be .object") + return + } + XCTAssertTrue(try XCTUnwrap(aCtx.properties["next"]).isReference) + XCTAssertTrue(try XCTUnwrap(bCtx.properties["next"]).isReference) + } } #endif