Skip to content

Commit fe60959

Browse files
authored
fix race condition when shutting down server
motivation: fix a race condition that can lead to crash and failing tests changes: close channel when canceling
1 parent d956b89 commit fe60959

File tree

3 files changed

+16
-7
lines changed

3 files changed

+16
-7
lines changed

Sources/AWSLambdaRuntimeCore/HTTPClient.swift

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -274,10 +274,15 @@ private final class UnaryHandler: ChannelDuplexHandler {
274274
preconditionFailure("invalid state, no pending request")
275275
}
276276

277-
// As defined in RFC 2616 Section 8.1.2:
278-
// [...] unless otherwise indicated, the client SHOULD assume
279-
// that the server will maintain a persistent connection, even
280-
// after error responses from the server.
277+
// As defined in RFC 7230 Section 6.3:
278+
// HTTP/1.1 defaults to the use of "persistent connections", allowing
279+
// multiple requests and responses to be carried over a single
280+
// connection. The "close" connection option is used to signal that a
281+
// connection will not persist after the current request/response. HTTP
282+
// implementations SHOULD support persistent connections.
283+
//
284+
// That's why we only assume the connection shall be closed if we receive
285+
// a "connection = close" header.
281286
let serverCloseConnection = response.headers.first(name: "connection")?.lowercased() == "close"
282287

283288
if !self.keepAlive || serverCloseConnection || response.version != .init(major: 1, minor: 1) {
@@ -308,6 +313,9 @@ private final class UnaryHandler: ChannelDuplexHandler {
308313
case is RequestCancelEvent:
309314
if self.pending != nil {
310315
self.completeWith(.failure(HTTPClient.Errors.cancelled))
316+
// after the cancel error has been send, we want to close the connection so
317+
// that no more packets can be read on this connection.
318+
_ = context.channel.close()
311319
}
312320
default:
313321
context.triggerUserOutboundEvent(event, promise: promise)

Sources/AWSLambdaRuntimeCore/LambdaLifecycle.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,17 +122,21 @@ extension Lambda {
122122
runner.run(logger: logger, handler: handler).whenComplete { result in
123123
switch result {
124124
case .success:
125+
logger.log(level: .debug, "lambda invocation sequence completed successfully")
125126
// recursive! per aws lambda runtime spec the polling requests are to be done one at a time
126127
_run(count + 1)
127128
case .failure(HTTPClient.Errors.cancelled):
128129
if case .shuttingdown = self.state {
129130
// if we ware shutting down, we expect to that the get next
130131
// invocation request might have been cancelled. For this reason we
131132
// succeed the promise here.
133+
logger.log(level: .info, "lambda invocation sequence has been cancelled for shutdown")
132134
return promise.succeed(count)
133135
}
136+
logger.log(level: .error, "lambda invocation sequence has been cancelled unexpectedly")
134137
promise.fail(HTTPClient.Errors.cancelled)
135138
case .failure(let error):
139+
logger.log(level: .error, "lambda invocation sequence completed with error: \(error)")
136140
promise.fail(error)
137141
}
138142
}

Sources/AWSLambdaRuntimeCore/LambdaRunner.swift

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,6 @@ extension Lambda {
6868
self.runtimeClient.reportResults(logger: logger, invocation: invocation, result: result).peekError { error in
6969
logger.error("could not report results to lambda runtime engine: \(error)")
7070
}
71-
}.always { result in
72-
// we are done!
73-
logger.log(level: result.successful ? .debug : .warning, "lambda invocation sequence completed \(result.successful ? "successfully" : "with failure")")
7471
}
7572
}
7673

0 commit comments

Comments
 (0)