Avoid warning when enabling strict concurrency check

[sebsto]

Avoid strict concurrency check warnings

Motivation:

My Lambda function is an actor to protect instances variables from race condition.

@main
actor MyLambda: LambdaHandler { ... }

However, when I compile with

swift build -Xswiftc -strict-concurrency=complete

I receive this warning

warning: non-sendable type 'Self.Event' in parameter of the protocol requirement satisfied by actor-isolated instance method 'handle(_:context:)' cannot cross actor boundary
    func handle(_ event: APIGatewayV2Request, context: AWSLambdaRuntimeCore.LambdaContext) async throws -> APIGatewayV2Response {

What I understand from the error message is that Event must be Sendable. But Event is an associated type on LambdaHandler, see
https://github.com/swift-server/swift-aws-lambda-runtime/blob/main/Sources/AWSLambdaRuntimeCore/LambdaHandler.swift#L146

In this case, it is an APIGatewayV2Request which correctly implements Sendable but it looks like the compiler can not detect that.

Modifications:

I changed all occurences of associatedtype Event and associatedtype Output in LambdaHandler.swift to add :Sendable.
I did not check for Swift >=5.6 because I understand Swift 5.7 is the minimum version to compile the runtime.

Result:

No compiler warnings when using Xswiftc -strict-concurrency=complete

[sebsto]

@tomerd FYI and approval. Let me know if #if swift(>=5.6) checks are required or not

[tomerd]

this seems reasonable to me. @fabianfett thoughts?

[ktoso]

looks sensible to me 👍

[sebsto]

It looks like there are some errors I did not capture locally.

First API change. Is there something I can do to tell the CI system this is indeed an intended API change ?

09:25:45 3 breaking changes detected in AWSLambdaRuntime:
09:25:45   💔 API breakage: extension SimpleLambdaHandler has generic signature change from  to <Self.Event : Swift.Sendable, Self.Output : Swift.Sendable>
09:25:45   💔 API breakage: extension LambdaHandler has generic signature change from  to <Self.Event : Swift.Sendable, Self.Output : Swift.Sendable>
09:25:45   💔 API breakage: extension EventLoopLambdaHandler has generic signature change from  to <Self.Event : Swift.Sendable, Self.Output : Swift.Sendable>
09:25:45 
09:25:45 3 breaking changes detected in AWSLambdaRuntimeCore:
09:25:45   💔 API breakage: protocol SimpleLambdaHandler has generic signature change from  to <Self.Event : Swift.Sendable, Self.Output : Swift.Sendable>
09:25:45   💔 API breakage: protocol LambdaHandler has generic signature change from  to <Self.Event : Swift.Sendable, Self.Output : Swift.Sendable>
09:25:45   💔 API breakage: protocol EventLoopLambdaHandler has generic signature change from  to <Self.Event : Swift.Sendable, Self.Output : Swift.Sendable>

The second set of errors seems to deal with strict concurrency checks. It is not specific to this branch / set of changes. The same errors are generated when building the main branch. I guess I should not care about these right now.

➜  swift-aws-lambda-runtime git:(main) ✗ docker-compose -f docker/docker-compose.yaml -f docker/docker-compose.al2.main.yaml -p swift-aws-lambda-runtime-swift-nightly-prb run --rm test
....
Building for debugging...
/code/Sources/AWSLambdaRuntimeCore/Terminator.swift:74:34: error: reference to captured var 'iterator' in concurrently-executing code; this is an error in the Swift 6 language mode
 72 |                     errors.append(error)
 73 |                 }
 74 |                 return terminate(iterator, errors: errors, promise: promise)
    |                                  `- error: reference to captured var 'iterator' in concurrently-executing code; this is an error in the Swift 6 language mode
 75 |             }
 76 |         }
...

Any idea how to address these ?

[fabianfett]

I don't think we should land this, as this is fundamentally a Swift issue. The problem here is that Sendable checking is too aggressive in 5.10, as the compiler does not understand transfer semantics around actors. This will be fixed in Swift 6, with region based isolation.

As this introduces a requirement on lambda handlers that is not needed, I'm not in support of this patch.

[fabianfett]

First API change. Is there something I can do to tell the CI system this is indeed an intended API change ?

This CI step isn't required. It is fine to be red. We use it as a warning signal. The only way to make it not red, is to not make breaking changes. But even then it sometimes has false positives.

[sebsto]

as per discussion with @fabianfett, let's not merge this change. It should be fixed in Swift 6.0