Files
swift-aws-lambda-runtime/Sources/AWSLambdaRuntime/HTTPServer/Lambda+LocalServer+Pool.swift
Sébastien StormacqandSebastien Stormacq 34e89b4027 Fix Test hangs in Lambda+LocalServer (#630) (#631)
# Fix test hangs caused by Pool cancellation race conditions

## Summary

This PR fixes two related race conditions in
`Lambda+LocalServer+Pool.swift` that were causing the test suite to hang
approximately 10% of the time.

## Problem

The test suite exhibited intermittent hangs (~10% frequency) due to two
bugs in the Pool implementation:

1. **Individual task cancellation bug**: When one task waiting for a
specific `requestId` was cancelled, the cancellation handler would
incorrectly cancel ALL waiting tasks instead of just the cancelled one.

2. **Server shutdown hang**: When the server shut down, waiting
continuations in the pools were never cancelled, causing handlers to
wait indefinitely for responses that would never arrive.

## Root Causes

### Root Cause #1: Cancellation Handler Removes ALL Continuations

The `onCancel` handler in `Pool._next()` was removing all continuations
from the `waitingForSpecific` dictionary when any single task was
cancelled:

```swift
onCancel: {
    // BUG: Removes ALL continuations, not just the cancelled task's
    for continuation in state.waitingForSpecific.values {
        toCancel.append(continuation)
    }
    state.waitingForSpecific.removeAll()
}
```

This caused unrelated concurrent invocations to fail with
`CancellationError` when one client cancelled their request.

### Root Cause #2: No Pool Cleanup During Server Shutdown

When the server shut down (e.g., test completes), the task group was
cancelled but the pools' waiting continuations were never notified. The
`/invoke` endpoint handlers would continue waiting for responses that
would never arrive because the Lambda function had stopped.

## Solution

### Fix #1: Only Remove Specific Continuation on Cancellation

Modified the cancellation handler to only remove the continuation for
the specific cancelled task:

```swift
onCancel: {
    // Only remove THIS task's continuation
    let continuationToCancel = self.lock.withLock { state -> CheckedContinuation<T, any Error>? in
        if let requestId = requestId {
            return state.waitingForSpecific.removeValue(forKey: requestId)
        } else {
            let cont = state.waitingForAny
            state.waitingForAny = nil
            return cont
        }
    }
    
    continuationToCancel?.resume(throwing: CancellationError())
}
```

### Fix #2: Add Pool Cleanup During Server Shutdown

Added `cancelAll()` method to the Pool class and call it during server
shutdown:

```swift
func cancelAll() {
    let continuationsToCancel = self.lock.withLock { state -> [CheckedContinuation<T, any Error>] in
        var toCancel: [CheckedContinuation<T, any Error>] = []
        
        if let continuation = state.waitingForAny {
            toCancel.append(continuation)
            state.waitingForAny = nil
        }
        
        for continuation in state.waitingForSpecific.values {
            toCancel.append(continuation)
        }
        state.waitingForSpecific.removeAll()
        
        return toCancel
    }
    
    for continuation in continuationsToCancel {
        continuation.resume(throwing: CancellationError())
    }
}
```

Called during server shutdown:

```swift
let serverOrHandlerResult1 = await group.next()!
group.cancelAll()

// Cancel all waiting continuations in the pools to prevent hangs
server.invocationPool.cancelAll()
server.responsePool.cancelAll()
```

## Changes

### Modified Files

- **Sources/AWSLambdaRuntime/HTTPServer/Lambda+LocalServer+Pool.swift**
- Fixed cancellation handler in `_next()` to only remove specific
continuation
  - Added `cancelAll()` method for server shutdown cleanup

- **Sources/AWSLambdaRuntime/HTTPServer/Lambda+LocalServer.swift**
  - Call `cancelAll()` on both pools during server shutdown

### New Files

- **Tests/AWSLambdaRuntimeTests/LocalServerPoolCancellationTests.swift**
  - Added comprehensive test suite with 3 tests
- `testCancellationOnlyAffectsOwnTask`: Verifies only the cancelled task
receives CancellationError
- `testConcurrentInvocationsWithCancellation`: Tests real-world scenario
with 5 concurrent invocations
- `testFIFOModeCancellation`: Ensures FIFO mode cancellation works
correctly

## Testing

### Before Fix
- Test suite hung ~10% of the time
- When 1 task was cancelled, all 5 concurrent tasks received
`CancellationError`
- Streaming tests would occasionally hang during shutdown

### After Fix
- All 91 tests pass consistently without hangs
- When 1 task is cancelled, only that specific task receives
`CancellationError`
- Other tasks continue waiting normally
- Server shutdown properly cleans up all waiting continuations
- Multiple consecutive test runs confirm stability

### Test Coverage

The new test suite reproduces both bugs and verifies the fixes:

1. **testCancellationOnlyAffectsOwnTask**: Creates 3 tasks waiting for
different requestIds, cancels only one, and verifies the others are not
affected
2. **testConcurrentInvocationsWithCancellation**: Simulates 5 concurrent
invocations with one cancellation
3. **testFIFOModeCancellation**: Tests FIFO mode to ensure it still
works correctly

---------

Co-authored-by: Sebastien Stormacq <stormacq@amazon.lu>
2026-01-15 20:58:03 +01:00

211 lines
8.9 KiB
Swift

//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftAWSLambdaRuntime open source project
//
// Copyright SwiftAWSLambdaRuntime project authors
// Copyright (c) Amazon.com, Inc. or its affiliates.
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftAWSLambdaRuntime project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
#if LocalServerSupport
import DequeModule
import Synchronization
@available(LambdaSwift 2.0, *)
extension LambdaHTTPServer {
/// A shared data structure to store the current invocation or response requests and the continuation objects.
/// This data structure is shared between instances of the HTTPHandler
/// (one instance to serve requests from the Lambda function and one instance to serve requests from the client invoking the lambda function).
internal final class Pool<T>: AsyncSequence, AsyncIteratorProtocol, Sendable where T: Sendable {
private let poolName: String
internal init(name: String = "Pool") { self.poolName = name }
typealias Element = T
struct State {
var buffer: Deque<T> = []
var waitingForAny: CheckedContinuation<T, any Error>?
var waitingForSpecific: [String: CheckedContinuation<T, any Error>] = [:]
}
private let lock = Mutex<State>(State())
/// enqueue an element, or give it back immediately to the iterator if it is waiting for an element
public func push(_ item: T) {
let continuationToResume = self.lock.withLock { state -> CheckedContinuation<T, any Error>? in
// First check if there's a waiting continuation that can handle this item
// Check for FIFO waiter first
if let continuation = state.waitingForAny {
state.waitingForAny = nil
return continuation
}
// Check for specific waiter
if let response = item as? LocalServerResponse,
let requestId = response.requestId,
let continuation = state.waitingForSpecific.removeValue(forKey: requestId)
{
return continuation
}
// No waiting continuation, add to buffer
state.buffer.append(item)
return nil
}
// Resume continuation outside the lock to prevent potential deadlocks
continuationToResume?.resume(returning: item)
}
/// Unified next() method that handles both FIFO and requestId-specific waiting
private func _next(for requestId: String?) async throws -> T {
// exit if the task is cancelled
guard !Task.isCancelled else {
throw CancellationError()
}
return try await withTaskCancellationHandler {
try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<T, any Error>) in
let nextAction: Result<T, PoolError>? = self.lock.withLock { state -> Result<T, PoolError>? in
if let requestId = requestId {
// Look for oldest (first) item for this requestId in buffer
if let index = state.buffer.firstIndex(where: { item in
if let response = item as? LocalServerResponse {
return response.requestId == requestId
}
return false
}) {
let item = state.buffer.remove(at: index)
return .success(item)
} else {
// Check for conflicting waiters
if state.waitingForAny != nil {
return .failure(PoolError(cause: .mixedWaitingModes))
}
if state.waitingForSpecific[requestId] != nil {
return .failure(PoolError(cause: .duplicateRequestIdWait(requestId)))
}
// No matching item, wait for it
state.waitingForSpecific[requestId] = continuation
return nil
}
} else {
// FIFO mode - take first item
if let first = state.buffer.popFirst() {
return .success(first)
} else {
// Check for conflicting waiters
if !state.waitingForSpecific.isEmpty {
return .failure(PoolError(cause: .mixedWaitingModes))
}
if state.waitingForAny != nil {
return .failure(PoolError(cause: .nextCalledTwice(state.waitingForAny!)))
}
state.waitingForAny = continuation
return nil
}
}
}
switch nextAction {
case .success(let item):
continuation.resume(returning: item)
case .failure(let error):
if case let .nextCalledTwice(prevContinuation) = error.cause {
prevContinuation.resume(throwing: error)
}
continuation.resume(throwing: error)
case .none:
// do nothing - continuation is stored in state
break
}
}
} onCancel: {
// Only remove THIS task's continuation
let continuationToCancel = self.lock.withLock { state -> CheckedContinuation<T, any Error>? in
if let requestId = requestId {
// Remove only the continuation for this specific requestId
return state.waitingForSpecific.removeValue(forKey: requestId)
} else {
// Remove only the FIFO continuation
let cont = state.waitingForAny
state.waitingForAny = nil
return cont
}
}
continuationToCancel?.resume(throwing: CancellationError())
}
}
/// Simple FIFO next() method - used by AsyncIteratorProtocol
func next() async throws -> T? {
try await _next(for: nil)
}
/// RequestId-specific next() method for LocalServerResponse - NOT part of AsyncIteratorProtocol
func next(for requestId: String) async throws -> T {
try await _next(for: requestId)
}
func makeAsyncIterator() -> Pool {
self
}
/// Cancel all waiting continuations - used during server shutdown
func cancelAll() {
let continuationsToCancel = self.lock.withLock { state -> [CheckedContinuation<T, any Error>] in
var toCancel: [CheckedContinuation<T, any Error>] = []
if let continuation = state.waitingForAny {
toCancel.append(continuation)
state.waitingForAny = nil
}
for continuation in state.waitingForSpecific.values {
toCancel.append(continuation)
}
state.waitingForSpecific.removeAll()
return toCancel
}
// Resume all continuations outside the lock
for continuation in continuationsToCancel {
continuation.resume(throwing: CancellationError())
}
}
struct PoolError: Error {
let cause: Cause
var message: String {
switch self.cause {
case .nextCalledTwice:
return "Concurrent invocations to next(). This is not allowed."
case .duplicateRequestIdWait(let requestId):
return "Already waiting for requestId: \(requestId)"
case .mixedWaitingModes:
return "Cannot mix FIFO waiting (next()) with specific waiting (next(for:))"
}
}
enum Cause {
case nextCalledTwice(CheckedContinuation<T, any Error>)
case duplicateRequestIdWait(String)
case mixedWaitingModes
}
}
}
}
#endif