fix(lambda): share HTTP sessions so URLSession teardown stops killing the process

.claude/rules/architecture.md

@@ -54,3 +54,28 @@ To add or change a collection's strategy:
 3. If the master needs format conversion (JP2/TIFF), route through `signedConverterURL`
    instead of decoding in-process — see [`converter.md`](converter.md).
 4. Validate end-to-end with `./Sources/Testing/CollectionTester/test-collection.sh "<Collection>"`.
+
+## HTTP client lifecycle
+
+All outbound HTTP goes through `NetworkRequestManager`
+(`Sources/NZImageApiLambda/Helpers/NetworkRequestManager.swift`), which uses Alamofire. Two rules,
+both there to keep the Lambda process alive:
+
+- **DigitalNZ requests** use Alamofire's global `AF` session.
+- **Everything else** (HTML scrapes, HEAD probes, ranged GET probes) uses one of three
+  process-lifetime `static let` `Session`s on `NetworkRequestManager`, differing only in request
+  timeout and whether they send `Range: bytes=0-0`.
+
+Nothing may create a `Session` (or a bare `URLSession`) per call. On Linux, releasing one runs
+`URLSession._MultiHandle.deinit` in swift-corelibs-foundation, which calls `curl_multi_cleanup`;
+that synchronously re-enters the registered `CURLMOPT_TIMERFUNCTION`, and for a zero timeout
+`updateTimeoutTimer(to: .immediate)` does `queue.async { nonisolatedSelf... }`, taking a strong
+reference to the object being deinitialized. The Swift runtime then aborts (or segfaults on the
+dangling reference), and Lambda reports `Runtime.ExitError` with a 500 even though the handler had
+already produced its answer. It is a race, so it only fires under scheduling pressure, which a
+512 MB Lambda has plenty of.
+
+To reproduce or re-verify, run the Lambda's local server in a Linux container under CPU pressure
+and hammer `POST /invoke`; a per-call-session build fails several requests in 40, a shared-session
+build fails none. `Tests/NZImageApiLambdaTests/NetworkRequestManagerSessionTests.swift` pins the
+shared-session invariant and each session's configuration.

CLAUDE.md

@@ -100,6 +100,14 @@ Secrets live in gitignored `.env`, `samconfig.toml`, and `.consumer-secrets/` 
 - `template.yaml` deliberately sets **no** `ReservedConcurrentExecutions` on either
   function (the account's Lambda concurrency quota is already only 10, fully unreserved) —
   don't describe concurrency as reserved/limited per-function.
+- **Never let a `URLSession` (or an Alamofire `Session`, which owns one) be deallocated in the
+  Lambda.** On Linux, `URLSession.deinit` tears down swift-corelibs-foundation's libcurl
+  `_MultiHandle`, whose own `deinit` re-enters curl's timer callback and takes a strong reference
+  to the object being deinitialized. The Swift runtime aborts the process, so a request that has
+  already done all its work returns a 500 (`Runtime.ExitError`). This caused an intermittent ~12-25%
+  failure rate on `/image` until 2026-08-24. `NetworkRequestManager` now holds process-lifetime
+  static `Session`s (`browserSession`, `shortTimeoutBrowserSession`, `rangeProbeSession`); add a new
+  static one there rather than constructing `Session(configuration:)` per call.
 - Routing a new collection through the converter requires adding its host to
   `ALLOWED_HOSTS` and redeploying the converter; pure `URLProcessor` strategy changes don't.
 - Compiled Swift binaries (e.g. running `CollectionLister` or `NZImageApiLambda` after

Sources/NZImageApiLambda/Helpers/NetworkRequestManager.swift

@@ -45,6 +45,67 @@ final class NetworkRequestManager: ValidatedRequestManager {
         return .success(())
     }
 
+    /// The browser User-Agent every non-DigitalNZ request presents. Several sources (Recollect
+    /// vanity domains, Te Papa media) 403 a request that looks like a bot.
+    static let browserUserAgent = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36"
+
+    /// Process-lifetime `Session`s, deliberately never released.
+    ///
+    /// These were previously created per call. On Linux, an Alamofire `Session` owns a
+    /// `URLSession`, whose `deinit` tears down swift-corelibs-foundation's libcurl
+    /// `URLSession._MultiHandle`. That teardown is unsound: `_MultiHandle.deinit` calls
+    /// `curl_multi_remove_handle`/`curl_multi_cleanup`, which synchronously re-enter the
+    /// registered `CURLMOPT_TIMERFUNCTION` callback; for a zero timeout that lands in
+    /// `updateTimeoutTimer(to: .immediate)`, which does `queue.async { nonisolatedSelf... }` and so
+    /// takes a *strong* reference to the object currently being deinitialized. The Swift runtime
+    /// then aborts the whole process with "Object ... of class _MultiHandle deallocated with
+    /// non-zero retain count". In a Lambda that abort surfaces as `Runtime.ExitError` and a 500,
+    /// after the request has already done all of its useful work.
+    ///
+    /// (Source: swift-corelibs-foundation, swift-6.3-RELEASE,
+    /// `Sources/FoundationNetworking/URLSession/libcurl/MultiHandle.swift`.)
+    ///
+    /// It is a race, so it only fires some of the time, and it fires far more often when CPU is
+    /// scarce, which is exactly a 512 MB Lambda. A session that is never deallocated never runs
+    /// that teardown, so keeping these alive for the life of the process removes the crash
+    /// entirely. Alamofire's own `AF` default session (used by `makeRequest`) is already a
+    /// process-lifetime global for the same reason.
+    ///
+    /// Do **not** reintroduce a per-call `Session(configuration:)` here.
+    static let browserSession = makeSession(
+        additionalHeaders: ["User-Agent": browserUserAgent],
+        requestTimeout: nil
+    )
+
+    /// As `browserSession`, but with the short timeout used by the redirect-following status probes.
+    static let shortTimeoutBrowserSession = makeSession(
+        additionalHeaders: ["User-Agent": browserUserAgent],
+        requestTimeout: 15
+    )
+
+    /// As `shortTimeoutBrowserSession`, plus the `Range: bytes=0-0` header that keeps the probing
+    /// GETs to a single byte.
+    static let rangeProbeSession = makeSession(
+        additionalHeaders: ["User-Agent": browserUserAgent, "Range": "bytes=0-0"],
+        requestTimeout: 15
+    )
+
+    private static func makeSession(
+        additionalHeaders: [String: String],
+        requestTimeout: TimeInterval?
+    )
+        -> Session
+    {
+        let configuration = URLSessionConfiguration.default
+        configuration.httpAdditionalHeaders = additionalHeaders
+
+        if let requestTimeout {
+            configuration.timeoutIntervalForRequest = requestTimeout
+        }
+
+        return Session(configuration: configuration)
+    }
+
     func makeRequest<ResponseType: NonNullableResult & Sendable>(
         endpoint: String,
         apiKey: String? = nil,
@@ -75,22 +136,15 @@ final class NetworkRequestManager: ValidatedRequestManager {
     }
 
     func fetchHTML(endpoint: String) async throws -> String {
-        let userAgent = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36"
-
-        let configuration = URLSessionConfiguration.default
-        configuration.httpAdditionalHeaders = ["User-Agent": userAgent]
-
-        let session = Session(configuration: configuration)
-
         // Also set the User-Agent as a per-REQUEST header (not only on the session config) so it is
         // carried onto the redirected URLRequest when URLSession auto-follows a redirect. Some
         // Recollect instances 301/302 the harvested *.recollect.co.nz landing host to a council vanity
         // domain (e.g. tasman.recollect.co.nz -> heritage.tasmanlibraries.govt.nz) that 403s any
         // request lacking a browser UA. Session-level httpAdditionalHeaders are NOT reliably reapplied
         // to the cross-host redirect (URLSession returns the 403 error page, so an og:image scrape sees
         // no image and falls back), whereas request headers ARE copied across the redirect.
-        let headers: HTTPHeaders = ["User-Agent": userAgent]
-        let response = await session.request(endpoint, headers: headers).serializingString().response
+        let headers: HTTPHeaders = ["User-Agent": Self.browserUserAgent]
+        let response = await Self.browserSession.request(endpoint, headers: headers).serializingString().response
 
         switch response.result {
         case let .success(value):
@@ -107,14 +161,10 @@ final class NetworkRequestManager: ValidatedRequestManager {
     /// safe at request time even for very large assets. Uses a browser User-Agent and a
     /// short timeout.
     func headStatusFollowingRedirects(endpoint: String) async -> Int {
-        let configuration = URLSessionConfiguration.default
-        configuration.httpAdditionalHeaders = [
-            "User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36"
-        ]
-        configuration.timeoutIntervalForRequest = 15
-
-        let session = Session(configuration: configuration)
-        let response = await session.request(endpoint, method: .head).serializingData().response
+        let response = await Self.shortTimeoutBrowserSession
+            .request(endpoint, method: .head)
+            .serializingData()
+            .response
 
         return response.response?.statusCode ?? 0
     }
@@ -127,15 +177,10 @@ final class NetworkRequestManager: ValidatedRequestManager {
     /// downloads the full image at request time (the endpoint must honour Range — Te Papa/S3 does).
     /// Browser User-Agent + short timeout.
     func rangeStatusFollowingRedirects(endpoint: String) async -> Int {
-        let configuration = URLSessionConfiguration.default
-        configuration.httpAdditionalHeaders = [
-            "User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36",
-            "Range": "bytes=0-0",
-        ]
-        configuration.timeoutIntervalForRequest = 15
-
-        let session = Session(configuration: configuration)
-        let response = await session.request(endpoint, method: .get).serializingData().response
+        let response = await Self.rangeProbeSession
+            .request(endpoint, method: .get)
+            .serializingData()
+            .response
 
         return response.response?.statusCode ?? 0
     }
@@ -146,29 +191,21 @@ final class NetworkRequestManager: ValidatedRequestManager {
     /// for GET only), but also surfaces the MIME type so a caller can branch on the resolved
     /// original's actual format (e.g. `image/jpeg` vs `image/tiff`) without downloading the body.
     func rangeContentType(endpoint: String) async -> (status: Int, contentType: String?) {
-        let configuration = URLSessionConfiguration.default
-        configuration.httpAdditionalHeaders = [
-            "User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36",
-            "Range": "bytes=0-0",
-        ]
-        configuration.timeoutIntervalForRequest = 15
-
-        let session = Session(configuration: configuration)
-        let response = await session.request(endpoint, method: .get).serializingData().response
+        let response = await Self.rangeProbeSession
+            .request(endpoint, method: .get)
+            .serializingData()
+            .response
 
         let status = response.response?.statusCode ?? 0
         let contentType = response.response?.value(forHTTPHeaderField: "Content-Type")
         return (status, contentType)
     }
 
     func headRequest(endpoint: String) async throws -> (contentType: String, contentLength: Int64) {
-        let configuration = URLSessionConfiguration.default
-        configuration.httpAdditionalHeaders = [
-            "User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36"
-        ]
-
-        let session = Session(configuration: configuration)
-        let response = await session.request(endpoint, method: .head).serializingData().response
+        let response = await Self.browserSession
+            .request(endpoint, method: .head)
+            .serializingData()
+            .response
 
         guard let httpResponse = response.response else {
             throw NetworkRequestManagerError(

Tests/NZImageApiLambdaTests/NetworkRequestManagerSessionTests.swift

@@ -0,0 +1,154 @@
+//
+//  NetworkRequestManagerSessionTests.swift
+//
+//  Regression coverage for the intermittent Lambda crash where a request completed all of its
+//  work and then the process aborted with:
+//
+//      Object 0x... of class _MultiHandle deallocated with non-zero retain count 2.
+//
+//  NetworkRequestManager used to build a fresh Alamofire `Session` on every call. On Linux that
+//  `Session` owns a `URLSession`, and releasing it runs swift-corelibs-foundation's
+//  `URLSession._MultiHandle.deinit`, which re-enters libcurl's timer callback and takes a strong
+//  reference to the object being deinitialized. The Swift runtime turns that into a fatal error,
+//  which API Gateway surfaced as a 500 on roughly one request in four.
+//
+//  These tests pin the fix: the sessions are process-lifetime shared instances (so nothing is ever
+//  deallocated), and each still carries the request configuration its callers depend on.
+//
+
+import Alamofire
+import Foundation
+import XCTest
+@testable import NZImageApiLambda
+
+final class NetworkRequestManagerSessionTests: XCTestCase {
+    // MARK: No per-call sessions
+
+    // The invariant this file exists to protect is "the Lambda target never constructs an HTTP
+    // session outside the shared static factory". That cannot be observed at runtime -- the
+    // statics are the same object however they are reached -- so it is checked against the source
+    // itself, which is what a regression would actually change.
+
+    func testTheLambdaTargetConstructsSessionsOnlyInTheSharedFactory() throws {
+        var offenders: [String] = []
+
+        for file in try Self.lambdaSourceFiles() {
+            let source = try String(contentsOf: file, encoding: .utf8)
+
+            for (offset, line) in Self.codeLines(of: source) {
+                guard line.contains("Session(configuration") || line.contains("URLSession(") else { continue }
+
+                // The one legitimate construction site.
+                let isSharedFactory = file.lastPathComponent == "NetworkRequestManager.swift"
+                    && line.contains("return Session(configuration: configuration)")
+
+                if !isSharedFactory {
+                    offenders.append("\(file.lastPathComponent):\(offset + 1): \(line.trimmingCharacters(in: .whitespaces))")
+                }
+            }
+        }
+
+        XCTAssertEqual(
+            offenders,
+            [],
+            """
+            A session is being constructed outside NetworkRequestManager.makeSession. On Linux, \
+            releasing a URLSession runs _MultiHandle.deinit, which aborts the process and turns a \
+            completed request into a 500. Add a process-lifetime static session instead.
+            """
+        )
+    }
+
+    func testTheSharedFactoryProducesExactlyTheThreeExpectedSessions() throws {
+        let source = try String(contentsOf: Self.networkRequestManagerSource(), encoding: .utf8)
+        let constructions = Self.codeLines(of: source).filter { $0.line.contains("Session(configuration") }
+        let factoryCalls = Self.codeLines(of: source).filter { $0.line.contains("makeSession(") }
+
+        XCTAssertEqual(constructions.count, 1, "Exactly one Session(configuration:) call is expected.")
+        // The three static properties, plus the factory's own declaration line.
+        XCTAssertEqual(factoryCalls.count, 4, "Expected three shared sessions built by one factory.")
+    }
+
+    // MARK: Configuration preserved
+
+    func testBrowserSessionSendsBrowserUserAgentAndNoRequestTimeoutOverride() {
+        let headers = Self.additionalHeaders(of: NetworkRequestManager.browserSession)
+
+        XCTAssertEqual(headers["User-Agent"], NetworkRequestManager.browserUserAgent)
+        XCTAssertNil(headers["Range"])
+        // Untouched, so it keeps URLSessionConfiguration.default's 60s.
+        XCTAssertEqual(NetworkRequestManager.browserSession.session.configuration.timeoutIntervalForRequest, 60)
+    }
+
+    func testShortTimeoutSessionKeepsTheFifteenSecondProbeTimeout() {
+        let headers = Self.additionalHeaders(of: NetworkRequestManager.shortTimeoutBrowserSession)
+
+        XCTAssertEqual(headers["User-Agent"], NetworkRequestManager.browserUserAgent)
+        XCTAssertNil(headers["Range"])
+        XCTAssertEqual(
+            NetworkRequestManager.shortTimeoutBrowserSession.session.configuration.timeoutIntervalForRequest,
+            15
+        )
+    }
+
+    func testRangeProbeSessionKeepsTheSingleByteRangeHeader() {
+        // Without this header the "does the high-res original exist" probes would download the
+        // whole asset instead of one byte.
+        let headers = Self.additionalHeaders(of: NetworkRequestManager.rangeProbeSession)
+
+        XCTAssertEqual(headers["User-Agent"], NetworkRequestManager.browserUserAgent)
+        XCTAssertEqual(headers["Range"], "bytes=0-0")
+        XCTAssertEqual(NetworkRequestManager.rangeProbeSession.session.configuration.timeoutIntervalForRequest, 15)
+    }
+
+    func testBrowserUserAgentLooksLikeABrowser() {
+        // Several sources 403 anything that does not present a browser UA.
+        XCTAssertTrue(NetworkRequestManager.browserUserAgent.hasPrefix("Mozilla/5.0"))
+    }
+
+    // MARK: Helpers
+
+    /// The repository root, derived from this file's own path.
+    private static func repositoryRoot() -> URL {
+        URL(fileURLWithPath: #filePath)
+            .deletingLastPathComponent() // NZImageApiLambdaTests
+            .deletingLastPathComponent() // Tests
+            .deletingLastPathComponent() // repository root
+    }
+
+    private static func networkRequestManagerSource() -> URL {
+        repositoryRoot()
+            .appendingPathComponent("Sources/NZImageApiLambda/Helpers/NetworkRequestManager.swift")
+    }
+
+    private static func lambdaSourceFiles() throws -> [URL] {
+        let root = repositoryRoot().appendingPathComponent("Sources/NZImageApiLambda")
+
+        guard let enumerator = FileManager.default.enumerator(at: root, includingPropertiesForKeys: nil) else {
+            throw NSError(domain: "NetworkRequestManagerSessionTests", code: 1)
+        }
+
+        return enumerator.compactMap { $0 as? URL }.filter { $0.pathExtension == "swift" }
+    }
+
+    /// Source lines with comment-only lines removed, so the doc comments that *mention*
+    /// `Session(configuration:)` do not register as constructions.
+    private static func codeLines(of source: String) -> [(offset: Int, line: String)] {
+        source
+            .components(separatedBy: "\n")
+            .enumerated()
+            .filter { !$0.element.trimmingCharacters(in: .whitespaces).hasPrefix("//") }
+            .map { (offset: $0.offset, line: $0.element) }
+    }
+
+    private static func additionalHeaders(of session: Session) -> [String: String] {
+        var headers: [String: String] = [:]
+
+        for (key, value) in session.session.configuration.httpAdditionalHeaders ?? [:] {
+            guard let key = key as? String, let value = value as? String else { continue }
+            headers[key] = value
+        }
+
+        return headers
+    }
+}