Skip to content

Commit

Permalink
Wait for closeFuture instead of close promise in NIOAsyncChannel'…
Browse files Browse the repository at this point in the history
…s `executeThenClose` (#3032)

After the changes introduced in
apple/swift-nio-http2#487, we need to make a
small change in the implementation of `NIOAsyncChannel` to wait on the
`closeFuture` instead of on `close`'s promise in the `executeThenClose`
implementation.

### Motivation:

`executeThenClose` shouldn't fail from errors arising from closing the
channel - at this point, the user of the channel cannot really do
anything, and since the channel has been closed, we should not fail
since resources have been cleaned up anyways.

### Modifications:

This PR changes the implementation of `NIOAsyncChannel` to wait on the
`closeFuture` instead of on `close`'s promise in the `executeThenClose`
implementation.
It also updates the docs for `closeFuture` to better explain when it
will be succeeded and why it won't ever be failed.


### Result:

`executeThenClose` won't throw errors upon closing.
  • Loading branch information
gjcairo authored Dec 17, 2024
1 parent 6008911 commit d73d862
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 6 deletions.
22 changes: 16 additions & 6 deletions Sources/NIOCore/AsyncChannel/AsyncChannel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -301,15 +301,25 @@ public struct NIOAsyncChannel<Inbound: Sendable, Outbound: Sendable>: Sendable {
}
}

self._outbound.finish()
// We ignore errors from close, since all we care about is that the channel has been closed
// at this point.
self.channel.close(promise: nil)
// `closeFuture` should never be failed, so we could ignore the error. However, do an
// assertionFailure to guide bad Channel implementations that are incorrectly failing this
// future to stop failing it.
do {
self._outbound.finish()
try await self.channel.close().get()
try await self.channel.closeFuture.get()
} catch {
if let error = error as? ChannelError, error == .alreadyClosed {
return result
}
throw error
assertionFailure(
"""
The channel's closeFuture should never be failed, but it was failed with error: \(error).
This is an error in the channel's implementation.
Refer to `Channel/closeFuture`'s documentation for more information.
"""
)
}

return result
}
}
Expand Down
6 changes: 6 additions & 0 deletions Sources/NIOCore/Channel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ public protocol Channel: AnyObject, ChannelOutboundInvoker, _NIOPreconcurrencySe
var allocator: ByteBufferAllocator { get }

/// The `closeFuture` will fire when the `Channel` has been closed.
///
/// - Important: This future should never be failed: it signals when the channel has been closed, and this action should not fail,
/// regardless of whether the close happenned cleanly or not.
/// If you are interested in any errors thrown during `close` to diagnose any unclean channel closures, you
/// should instead use the future returned from ``ChannelOutboundInvoker/close(mode:file:line:)-7hlgf``
/// or pass a promise via ``ChannelOutboundInvoker/close(mode:promise:)``.
var closeFuture: EventLoopFuture<Void> { get }

/// The `ChannelPipeline` which handles all I/O events and requests associated with this `Channel`.
Expand Down

0 comments on commit d73d862

Please sign in to comment.