Propagate URLSession errors to HTTPBodyOutputStreamBridge#94
Conversation
| deinit { | ||
| debug("Output stream delegate deinit") | ||
| outputStream.delegate = nil | ||
| if outputStream.streamStatus != .notOpen && outputStream.streamStatus != .closed { |
There was a problem hiding this comment.
Added this while debugging. Shouldn't really be necessary any more with the other changes but I think it's a good defensive check
There was a problem hiding this comment.
Most NSStream APIs are not thread-safe but deinit can run on any thread, so it is not safe to call its methods here.
There was a problem hiding this comment.
Will just remove. Will have to drop the unit test for this too.
| deinit { | ||
| debug("Output stream delegate deinit") | ||
| outputStream.delegate = nil | ||
| if outputStream.streamStatus != .notOpen && outputStream.streamStatus != .closed { |
There was a problem hiding this comment.
Most NSStream APIs are not thread-safe but deinit can run on any thread, so it is not safe to call its methods here.
|
@guoye-zhang Updated per your feedback |
| mutating func errorOccurred(_ error: any Error) -> Action { | ||
| switch self { | ||
| case .initial: | ||
| self = .closed(error) |
There was a problem hiding this comment.
Is this change necessary?
There was a problem hiding this comment.
I believe it is. In the initializer, the output stream is opened and the state is set to initial. If URLSession receives an error before the state updates then the output stream is never closed.
2 participants
Motivation
Resolves leaks described in swift-openapi-generator#917 by explicitly passing
URLSessionerrors on toHTTPBodyOutputStreamBridgefor proper cleanup.Modifications
Added a
cancel(error)method toHTTPBodyOutputStreamBridgewhichBidirectionalStreamingURLSessionDelegatecalls directly when handling aURLSessionerror.Result
The
outputStreamis closed onURLSessionerrors and the writerTaskis cancelled if it was running to avoid keeping a strong reference toself.Test Plan