Add a server connection manager by aryan-25 · Pull Request #532 · apple/swift-nio-http2

aryan-25 · 2026-02-03T11:48:18Z

Motivation:

NIOHTTP2Handler currently does not support graceful shutdown because it does not action upon a ChannelShouldQuiesceEvent event. See issue #336.

Per the comment on that issue, gRPC-swift-nio-transport implements this functionality in a separate channel handler, named ServerConnectionManagementHandler. We should introduce this to this repo too.

Modifications:

Added ServerConnectionManagementHandler from gRPC-swift-nio-transport with some slight modifications such as removing gRPC-specific functionality like policing client pings.
Added a new configureAsyncHTTP2Pipeline helper, which takes the connection manager handler as an argument and sets it up alongside NIOHTTP2Handler.

Result:

Addresses issue #336.

glbrntt

Could you add the unmodified code from gRPC as the first commit in this PR to make it easier to see the changes you've made?

Sources/NIOHTTP2/Timer.swift

…C-swift-nio-transport

glbrntt · 2026-02-05T10:17:51Z

Sources/NIOHTTP2/NIOHTTP2ServerConnectionManagementHandler.swift

+    /// While NIO's `EmbeddedEventLoop` provides control over its view of time (and therefore any
+    /// events scheduled on it) it doesn't offer a way to get the current time. This is usually done
+    /// via `NIODeadline`.
+    public struct Clock {


I would err on the side of caution and keep this internal/package because it was the minimal API required to enable various tests in gRPC. We can always think a bit harder about the API and make it public in the future if necessary.

glbrntt · 2026-02-05T10:42:41Z

Sources/NIOHTTP2/NIOHTTP2ServerConnectionManagementHandler.swift

+    ///   - keepaliveConfiguration: Configuration for keep-alive ping behavior. Defaults to `nil`, disabling keep-alive
+    ///     pings.
+    ///   - clock: A clock providing the current time.
+    public init(


I recommend adding a config struct for all of these values: it will make API evolution much easier (e.g. if more config is added later). Also means you can provide a set of sensible default values as a static computed property on that config too.

Agreed. I've also added a default configuration property in 92aa96e:

public static var defaults: Self { Self( maxIdleTime: nil, maxAge: nil, maxGraceTime: .minutes(5), keepalive: .init(pingInterval: .hours(2), ackTimeout: .seconds(20)) ) }

glbrntt · 2026-02-06T08:27:15Z

Sources/NIOHTTP2/HTTP2PipelineHelpers.swift

+    @available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *)
+    public func configureAsyncHTTP2Pipeline<Output: Sendable>(
+        mode: NIOHTTP2Handler.ParserMode,
+        connectionManager: NIOHTTP2ServerConnectionManagementHandler,


Passing in the handler is a bit odd. It'd be more idiomatic to pass in the config for the handler and have this func create the handler for the caller.

Lukasa · 2026-02-06T12:18:24Z

Sources/NIOHTTP2/Timer.swift

+    }
+}
+
+extension Timer: NIOScheduledCallbackHandler, @unchecked Sendable where Handler: Sendable {


This unchecked sendable seems wrong: it relies on correct use. Can we make it actually Sendable?

Lukasa · 2026-02-06T12:20:04Z

Sources/NIOHTTP2/NIOHTTP2ServerConnectionManagementHandler.swift

+        }
+
+        /// A manual clock for testing that allows explicit control over time.
+        public final class Manual {


These publics don’t do anything, so we can safely remove them.

Lukasa · 2026-02-06T12:21:50Z

Sources/NIOHTTP2/NIOHTTP2ServerConnectionManagementHandler.swift

+    }
+
+    public func handlerAdded(context: ChannelHandlerContext) {
+        assert(context.eventLoop === self.eventLoop)


Replace this assert with an in-event-loop assertion.

Lukasa · 2026-02-06T12:24:32Z

Sources/NIOHTTP2/NIOHTTP2ServerConnectionManagementHandler.swift

+
+// Timer handler views.
+extension NIOHTTP2ServerConnectionManagementHandler {
+    struct MaxIdleTimerHandlerView: @unchecked Sendable, NIOScheduledCallbackHandler {


What’s the need for these to be Sendable? If we do actually require it, we should fully enforce it using proper preconditions or loop bounds.

In Timer, we call the EventLoop/scheduleCallback(at:handler:) function with the handler argument set to the view.

The scheduleCallback function requires handler to be Sendable, which is why we need to make these views Sendable.

Lukasa · 2026-02-06T12:25:18Z

Sources/NIOHTTP2/NIOHTTP2ServerConnectionManagementHandler.swift

+
+extension NIOHTTP2ServerConnectionManagementHandler {
+    /// A delegate for receiving HTTP/2 stream lifecycle events.
+    public struct HTTP2StreamDelegate: @unchecked Sendable, NIOHTTP2StreamDelegate {


Why is this public? Same note here re sendability as well.

NIOHTTP2ServerConnectionManagementHandler will only know when streams are opened / closed via its delegate; users need to pass this stream delegate to NIOHTTP2Handler for NIOHTTP2ServerConnectionManagementHandler to work as intended.

This reason this is public is so users can also use NIOHTTP2ServerConnectionManagementHandler without this pipeline setup helper.

…ndable`.

Lukasa · 2026-02-09T13:50:56Z

Sources/NIOHTTP2/NIOHTTP2ServerConnectionManagementHandler.swift

+    }
+
+    /// Configuration parameters for ``NIOHTTP2ServerConnectionManagementHandler``.
+    public struct Configuration: Sendable {


This is a sufficiently large range of configuration knobs that it might be nice for us to add some narrative documentation explaining what this thing is for, how it works, and when you might use it.

Lukasa · 2026-02-09T13:56:33Z

Sources/NIOHTTP2/NIOHTTP2ServerConnectionManagementHandler.swift

+            ///   - pingInterval: The amount of time to wait after reading data before sending a keep-alive ping.
+            ///   - ackTimeout: The amount of time the client has to reply after the server sends a keep-alive ping to keep
+            ///     the connection open. The connection is closed if no reply is received.
+            public init(pingInterval: TimeAmount, ackTimeout: TimeAmount = .seconds(20)) {


What's the principle behind defaulting only the second parameter?

In gRPC-swift-nio-transport where this connection manager is derived from, both the pingInterval and ackTimeout arguments are contained in the top-level config type, not in a standalone Keepalive type like in this PR.

It is possible (in gRPC-swift-nio-transport) to set pingInterval to a non-nil value and ackTimeout to nil. In that case, a default of 20 seconds is used for ackTimeout.

I can remove the default argument for ackTimeout here since that case does not arise: users must define an ackTimeout when pingTimeout is non-nil.

Lukasa · 2026-02-09T13:58:19Z

Sources/NIOHTTP2/Timer.swift

+
+    /// Start or restart the timer.
+    mutating func start() {
+        self.eventLoop.assertInEventLoop()


These assertions are weird now that this type might be Sendable. Are they actually required in any way for correct function? It seems like probably not.

Lukasa · 2026-02-09T14:06:58Z

Sources/NIOHTTP2/NIOHTTP2ServerConnectionManagementHandler.swift

+        context.fireUserInboundEventTriggered(event)
+    }
+
+    public func errorCaught(context: ChannelHandlerContext, error: any Error) {


Is it appropriate for us to suppress errors here?

We should not be determining whether to close the connection here, and just be propagating the error down the pipeline.

I've removed the errorCaught function entirely.

Lukasa · 2026-02-09T14:08:42Z

Sources/NIOHTTP2/NIOHTTP2ServerConnectionManagementHandler.swift

+        self.inReadLoop = false
+
+        // Done reading: schedule the keep-alive timer.
+        self.keepaliveTimerHandler?.start()


Is this logic entirely right? We can get a channelReadComplete with no actual read data getting to us here. Is the semantic that we need a read of anything from the network, even if it never produced a frame, or do we need a complete frame?

I think any activity on the network originating from the client is sufficient for us to reset the keepalive timer.

@glbrntt, what are your thoughts on this? The gRPC proposal, which I believe the connection manager implementation in gRPC-swift-nio-transport closely follows, states this:

"keepalive time is ideally measured from the time of the last byte read"

Note: The above is actually from the client-side connection management proposal (the server-side proposal refers readers to the client-side proposal for implementing keepalive checks (see this)).

We don't need a complete frame, just any bytes on the network if we're following the gRPC semantics.

Lukasa · 2026-02-09T14:12:46Z

Sources/NIOHTTP2/NIOHTTP2ServerConnectionManagementHandler.swift

+// Timer handler views.
+extension NIOHTTP2ServerConnectionManagementHandler {
+    struct MaxIdleTimerHandlerView: Sendable, NIOScheduledCallbackHandler {
+        private let handler: NIOLoopBound<NIOHTTP2ServerConnectionManagementHandler>


It's still not clear to me why these views need to be Sendable. There's no requirement for it that I can see.

Sorry, I forgot to answer this in your previous comment. See #532 (comment).

… errors down the pipeline.

glbrntt · 2026-02-16T11:17:05Z

Sources/NIOHTTP2/NIOHTTP2ServerConnectionManagementHandler+StateMachine.swift

+            /// Send a GOAWAY frame with the code "enhance your calm" and immediately close the connection.
+            case enhanceYourCalmThenClose(HTTP2StreamID)


This is never used because the keepalive policing was removed

glbrntt · 2026-02-16T11:20:15Z

Sources/NIOHTTP2/NIOHTTP2ServerConnectionManagementHandler.swift

+        /// Creates a clock with a manual time source for testing.
+        /// - Parameter time: The manual time source.
+        /// - Returns: A clock that uses the provided manual time source.
+        static func manual(_ time: Manual) -> Self {
+            Self(base: .manual(time))
+        }
+
+        /// A manual clock for testing that allows explicit control over time.
+        final class Manual {
+            private(set) var time: NIODeadline
+
+            /// Creates a manual clock with time starting at zero.
+            init() {
+                self.time = .uptimeNanoseconds(0)
+            }
+
+            func advance(by amount: TimeAmount) {
+                self.time = self.time + amount
+            }
+        }


Looks like this is never used, even by the tests.

glbrntt

LGTM although it'd be good to add the note about why we can ignore pings (but not ping acks)

glbrntt · 2026-02-17T10:35:21Z

Sources/NIOHTTP2/NIOHTTP2ServerConnectionManagementHandler.swift

+            self.handlePingAck(context: context, data: data)
+
+        default:
+            ()  // Only interested in PING frames with the ACK flag set, ignore the rest.


It's worth adding a note here that the HTTP2Handler acks PING frames and that's why we can ignore them here.

… not set

aryan-25 added the 🆕 semver/minor Adds new public API. label Feb 3, 2026

glbrntt requested changes Feb 5, 2026

View reviewed changes

Sources/NIOHTTP2/Timer.swift Show resolved Hide resolved

aryan-25 added 4 commits February 5, 2026 09:57

Bring ServerConnectionManagementHandler and associated files from gRP…

0e06179

…C-swift-nio-transport

Add a server connection manager

be27e7b

Update documentation

a4f06b6

Add documentation for configureAsyncHTTP2Pipeline

ce91534

aryan-25 force-pushed the server-connection-manager branch from edf43a7 to ce91534 Compare February 5, 2026 10:00

aryan-25 added 2 commits February 5, 2026 10:41

Add NIOHTTP2 prefix to file names

0b845ab

Merge branch 'main' into server-connection-manager

be04127

glbrntt reviewed Feb 6, 2026

View reviewed changes

Address feedback from review

ecb5630

Lukasa reviewed Feb 6, 2026

View reviewed changes

aryan-25 added 8 commits February 8, 2026 22:44

Remove redundant public modifiers

f9377f4

Remove repeating property from Timer; make Timer conform to `Se…

c60a29a

…ndable`.

Make handler views Sendable

7dcde4c

Make stream delegate Sendable

3b22bd2

Replace assert check with preconditionInEventLoop()

3123363

Remove redundant func

112133c

Merge branch 'main' into server-connection-manager

825f70b

Suppress NoBlockComments formatting errors

8286cd7

Lukasa reviewed Feb 9, 2026

View reviewed changes

aryan-25 mentioned this pull request Feb 9, 2026

Add support for swift-service-lifecycle swift-server/swift-http-server#55

Merged

aryan-25 added 8 commits February 10, 2026 12:29

Update documentation for Configuration

03f8720

Remove default argument for ackTimeout

9a9ebec

Propagate error forward in errorCaught

c50b7a4

Remove errorCaught() function; let default implementation propagate…

1b0a455

… errors down the pipeline.

Remove in-event-loop assertions from Timer

b564fc0

Add a default configuration property

92aa96e

Merge branch 'main' into server-connection-manager

fe56484

Run formatter

945121b

aryan-25 requested review from Lukasa and glbrntt February 13, 2026 14:02

glbrntt reviewed Feb 16, 2026

View reviewed changes

aryan-25 added 3 commits February 16, 2026 16:33

Remove handling of ping where ack flag not set

d681526

Remove Clock; not used anywhere

23700d4

Remove dead code

99787fb

glbrntt approved these changes Feb 17, 2026

View reviewed changes

Add comment about NIOHTTP2Handler handling PING frames where ACK flag…

3d6e195

… not set

aryan-25 merged commit b6571f3 into apple:main Feb 18, 2026
50 checks passed

aryan-25 mentioned this pull request Feb 18, 2026

NIOHTTP2Handler should handle ChannelShouldQuiesceEvent #336

Closed

aryan-25 linked an issue Feb 19, 2026 that may be closed by this pull request

NIOHTTP2Handler should handle ChannelShouldQuiesceEvent #336

Closed

		/// Send a GOAWAY frame with the code "enhance your calm" and immediately close the connection.
		case enhanceYourCalmThenClose(HTTP2StreamID)

Conversation

aryan-25 commented Feb 3, 2026

Uh oh!

glbrntt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aryan-25 Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glbrntt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aryan-25 Feb 9, 2026 •

edited

Loading