Add structured concurrency aware ServerBootstrap bind #3378

fabianfett · 2025-09-22T10:14:22Z

This is a very early draft of a structured concurrency aware ServerBootstrap bind method.

To be discussed:

How can we gracefully exit this method without adding a ServiceLifecycle dependency?

Sources/NIOPosix/Bootstrap.swift

FranzBusch · 2025-09-22T10:20:13Z

Sources/NIOPosix/Bootstrap.swift

+        _ onConnection: @escaping @Sendable (
+            _ inbound: NIOAsyncChannelInboundStream<Inbound>,
+            _ outbound: NIOAsyncChannelOutboundWriter<Outbound>


We should give the user the entire NIOAsyncChannel instead of just the two halfs. They might wanna access the underlying channel which they can through the asyncChannel.channel property

FranzBusch · 2025-09-22T10:21:04Z

Sources/NIOPosix/Bootstrap.swift

+        _ onConnection: @escaping @Sendable (
+            _ inbound: NIOAsyncChannelInboundStream<Inbound>,
+            _ outbound: NIOAsyncChannelOutboundWriter<Outbound>
+        ) async throws -> ()


Not sure if this method should be throwing and if we should use a throwing task group here. Right now those errors will lead to the task group getting cancelled which doesn't seam right to me? At least we should document what happens.

Sources/NIOPosix/Bootstrap.swift

FranzBusch · 2025-09-22T14:04:59Z

Sources/NIOPosix/Bootstrap.swift

+        /// specifies the exact binding location, including IPv4, IPv6, or Unix domain addresses.
+        ///
+        /// - Parameter address: The socket address to bind to
+        public static func address(_ address: SocketAddress) -> BindTarget {


NIT: Since it might be confused with a DNS address

Suggested change

public static func address(_ address: SocketAddress) -> BindTarget {

public static func socketAddress(_ socketAddress: SocketAddress) -> BindTarget {

Sources/NIOPosix/Bootstrap.swift

glbrntt · 2025-09-22T15:20:15Z

Sources/NIOPosix/Bootstrap.swift

+    /// - Note: The bind method respects task cancellation which will force close the server. If you want to gracefully
+    ///         shut-down use the quiescing helper approach as outlined above.
+    @available(macOS 14.0, iOS 17.0, watchOS 10.0, tvOS 17.0, *)
+    public func bind<Inbound: Sendable, Outbound: Sendable>(


If I implement a server using this method, how can I access the address that the listening socket was bound to? (e.g. I set port to zero and want to know the actual port I got.)

Currently it seems the only way to do this is via the parent channel of an accepted connection. This seems like a pretty big API deficiency.

I think that's a good point and we probably need another closure that is called once the server channel is bound.

@glbrntt I've added a onceStartup closure. Is this sufficient for you @glbrntt?

I think it should be async (see other comment) and I'm not keen on the name. Maybe something along the lines of onTCPListener / onListeningChannel? That'd at least be consistent with onConnection.

I agree. I think actually it should be async and throws since it is similar to a with-style method i.e. it runs in the calling task. I also like with the onXXX naming. To throw in one more alternative onListeningChannelBound

went for onListeningChannel and handleConnection (as @FranzBusch suggested somewhere else). Please bikeshed! @glbrntt @FranzBusch @Lukasa

I would bias towards some level of consistency.

We already have the established {server,child}ChannelInitializer pattern and we use childChannelInitializer in this signature already.

So perhaps we should have:

handleServerChannel and

handleChildChannel

This has clear parallels with the existing bootstrap APIs.

@glbrntt love it!

glbrntt · 2025-09-22T15:24:46Z

Sources/NIOPosix/Bootstrap.swift

+                                        try await connectionChannel.executeThenClose { _, _ in
+                                            await onConnection(connectionChannel)
+                                        }


I understand the rationale for passing in the NIOAsyncChannel but if we use this approach the API becomes pretty confusing:

If I use this structured API I mustn't call executeThenClose and I'm expected to use inbound and outbound which are no longer deprecated

If I use another bind method which isn't structured then I must call executeThenClose and shouldn't use inbound and outbound

I think the usability here needs a little more thought. We could just pass in the inbound, outbound and channel.

My thought process here was that initially when we designed the NIOAsyncChannel we wanted folks to use inbound and outbound. Then we realized our bootstraps aren't structured and we added the executeThenClose method. Now with the structured bootstraps we can go back to inbound/outbound. However, we need to tell one coherent story so I would argue to deprecate all the other bind methods + executeThenClose and push users to these new structured APIs.

We do have to consider the ecosystem churn this produces but I personally would push on us deprecating the non-structured bootstrap methods.

@glbrntt I think I'm with Franz here. Does this approach work for you as well?

I'm okay with that if we can first prove out that the new APIs (not just the server one) are suitable replacements. I think we burned some good will with the first deprecate and replace and so we should be really sure that these APIs are going to stick.

To that end it might be worth adding the new APIs as underscored and holding off on removing the deprecation warning on the inbound and outbound.

I'm not sure that I agree with that. Our view on approaching structured concurrency has significantly evolved in the last two years. The initial set of APIs was introduced in June 2023. So we change those APIs after two years. Who will adopt those APIs if they are underscored so that we can get feedback from the new APIs?

The point is we introduced the APIs and then changed them.

Before we move to the third iteration of this API we should be really damn sure that the model we land on works and is coherent for both server, client and multiplexing.

It being underscored gives us a chance to try it out (e.g. in gRPC, Vapor) without fully committing to it.

Joannis · 2025-09-22T17:53:52Z

Sources/NIOPosix/Bootstrap.swift

+                                }
+                            }
+                        }
+                        return .success(())


Does this implementation need to know about quiescing and/or graceful shutdown?

What's the expected behavior when a server stops accepting clients I.E. ServerBootstrap closes? Do all child tasks cancel, or do we wait for them to shut down? This kinda plays into graceful shutdown from Service Lifecycle. Can a connection continue to live while the parent channel is closed?

glbrntt · 2025-09-23T10:23:31Z

Sources/NIOPosix/Bootstrap.swift

+            childChannelInitializer: childChannelInitializer
+        )
+
+        onceStartup(channel.channel)


I think this should be part of the task group instead of before it.

Users can modify the server channel pipeline in the bootstrap so they should be able to interact with it while the server is running (maybe they're waiting for a signal before firing something down the pipeline).

I'm not sure I'm following. When you say part of the task group you mean moving it into the beginning of the withDiscardingTaskGroup or into a child task?

Sorry, that was unclear, I meant as a child task.

I'm not sure I agree with this since it significantly changes the composability of the closure. When it moves into a child task then it must become sending at least which means you can't interact with any task isolated state. Let's take a common server example

final class HTTPServer { var state = "starting" func run() async throws { try await ServerBootstrap.bind( target: .hostAndPort(self.host, self.port), onListeningChannelBound: { channel in self.state = "bound: \(channel.localAddress!)" } onConnection: { ... } ) } }

If it moves into a child task then onListeningChannelBound: { channel in self.state = "bound: \(channel.localAddress!)" } will need the state to move into a Mutex or use another synchronization mechanism.

This means that you can only use the channel before the server starts processing requests though.

What if I want to later send an event into the server channel?

Isn't the intent of these APIs to use structured concurrency?

@FranzBusch but you 100% need a lock around that.

The code I pasted above is data race safe. Channel is Sendable so you are allowed to store it in your local property. Also the closure is not sending or @Sendable.

Isn't the intent of these APIs to use structured concurrency?

I hear you and I can buy into saying that this is the reason that it needs to be called within a child task but that child tasks is going to introduce new race conditions such as a connection being handled before the onListeningChannelBound closure is called. So maybe we need both?

Hmm, maybe? I don't love giving two separate closures here, I think it makes the API more confusing than it should be.

When would the race matter? Anything that needs to be applied before channels are accepted should be done in the channel initializer.

@Lukasa do you have an opinion here?

I wonder if the tension is best addressed in a different way.

The serverChannelInitializer can be used for unstructured access to the server channel, and it can be used to escape the server channel reference as needed, though it does require a lock to do it. There is no way to get access to the server channel before you have bound it other than that spelling, and that spelling isn't structured. We can choose to make it so, but I'm honestly not sure it's the right answer.

However, having an async closure to operate on the server channel does make sense. This task provides a place to do asynchronous operations that may want to monitor the server channel in various ways. So my instinct is that I am leaning towards George's proposal, with the suggestion that we should document the behaviour required for non-structured access to the server channel.

In the event that we want a "before active" step, that's still something we can add later, and should probably have that specific name to indicate its lifecycle behaviour.

glbrntt · 2025-09-23T10:25:58Z

Sources/NIOPosix/Bootstrap.swift

+    /// - Note: The bind method respects task cancellation which will force close the server. If you want to gracefully
+    ///         shut-down use the quiescing helper approach as outlined above.
+    @available(macOS 14.0, iOS 17.0, watchOS 10.0, tvOS 17.0, *)
+    public func bind<Inbound: Sendable, Outbound: Sendable>(


I think it should be async (see other comment) and I'm not keen on the name. Maybe something along the lines of onTCPListener / onListeningChannel? That'd at least be consistent with onConnection.

Sources/NIOPosix/Bootstrap.swift

Lukasa · 2025-09-24T09:10:54Z

Sources/NIOCore/AsyncChannel/AsyncChannel.swift

    }
+
    /// The writer for writing outbound messages.
-    @available(*, deprecated, message: "Use the executeThenClose scoped method instead.")


Why are we undeprecating these?

because we want to pass in the NIOAsyncChannel and users shall then use inbound and outbound on it. So that potentially we can add further properties to NIOAsyncChannel later.

Can we undeprecate with an spi?

No you can't. What you can do is put it behind a trait like _ExperimentalStructuredBootstrap. You just have to duplicate the properties.

should I intro new in and out properties and mark them with the same @_spi?

Sources/NIOPosix/Bootstrap.swift

glbrntt · 2025-09-25T12:05:59Z

Sources/NIOPosix/Bootstrap.swift

+    ///    .serverChannelOption(.socketOption(.so_reuseaddr), value: 1)
+    ///    .bind(
+    ///        target: .hostAndPort(self.host, self.port),
+    ///        childChannelInitializer: { channel in


Can you use multiple trailing closure syntax here?

Yes, this is done in the NIOTCPEchoExample which is part of this pr.

glbrntt · 2025-09-25T12:06:26Z

Sources/NIOPosix/Bootstrap.swift

+    ///    }
+    ///    .serverChannelOption(.socketOption(.so_reuseaddr), value: 1)
+    ///    .bind(
+    ///        target: .hostAndPort(self.host, self.port),


Should the label here be to:? e.g. bind(to: .hostAndPort("foo", 0))

I've discussed this with @Lukasa, and he wants this to be target, as this type might change in the future. With a special label we can deprecate much better, than with a to: label.

glbrntt · 2025-09-25T12:08:20Z

Sources/NIOPosix/Bootstrap.swift

+    ///   - handleConnection: A closure to handle the connection. Use the channel's `inbound` property to read from
+    ///                       the connection and channel's `outbound` to write to the connection.
+    ///   - onListeningChannel: A closure that will be called once the server has been started. Use this to get access to
+    ///                         the serverChannel, if you used port `0` in the ``BindTarget``. You can also use it to


This makes it sound like you only get access if you set the port to 0.

Tried to reworded it.

glbrntt · 2025-09-25T12:10:30Z

Sources/NIOPosix/Bootstrap.swift

+            _ channel: NIOAsyncChannel<Inbound, Outbound>
+        ) async -> (),
+        onListeningChannel: @Sendable @escaping (
+            NIOAsyncChannel<NIOAsyncChannel<Inbound, Outbound>, Never>


There are a lot of documented constraints on this parameter. It feels like a very odd design decision to pass the user a NIOAsyncChannel on which they can't use inbound/outbound/executeThenClose. Why don't we just provide them with a Channel (or some other type)?

I agree that in this case it is best to pass the plain Channel.

glbrntt · 2025-09-25T12:16:02Z

Sources/NIOPosix/Bootstrap.swift

+    /// - Note: The bind method respects task cancellation which will force close the server. If you want to gracefully
+    ///         shut-down use the quiescing helper approach as outlined above.
+    @available(macOS 14.0, iOS 17.0, watchOS 10.0, tvOS 17.0, *)
+    public func bind<Inbound: Sendable, Outbound: Sendable>(


I would bias towards some level of consistency.

We already have the established {server,child}ChannelInitializer pattern and we use childChannelInitializer in this signature already.

So perhaps we should have:

handleServerChannel and

handleChildChannel

This has clear parallels with the existing bootstrap APIs.

fabianfett requested a review from FranzBusch September 22, 2025 10:14

FranzBusch reviewed Sep 22, 2025

View reviewed changes

fabianfett requested a review from FranzBusch September 22, 2025 13:51

FranzBusch reviewed Sep 22, 2025

View reviewed changes

glbrntt reviewed Sep 22, 2025

View reviewed changes

Joannis reviewed Sep 22, 2025

View reviewed changes

fabianfett added 7 commits September 23, 2025 11:59

Add structured concurrency aware ServerBootstrap bind

76c7590

Undeprecate NIOAsyncChannel properties

70b0aa1

Doc how to graceful shutdown

afa3d9d

Update NIOTCPEchoServer example

8dbb961

PR review: Use non throwing task group.

227beb8

PR review

21cd496

Add once startup closure

c05579c

fabianfett force-pushed the ff-structured-ServerChannel-bind branch from e637823 to c05579c Compare September 23, 2025 10:00

glbrntt reviewed Sep 23, 2025

View reviewed changes

FranzBusch reviewed Sep 23, 2025

View reviewed changes

Sources/NIOPosix/Bootstrap.swift Outdated Show resolved Hide resolved

Lukasa reviewed Sep 24, 2025

View reviewed changes

fabianfett added 2 commits September 25, 2025 11:46

PR reviews

378dca5

Use spi

1b3dde7

glbrntt reviewed Sep 25, 2025

View reviewed changes

fabianfett added 6 commits September 25, 2025 15:33

PR review

cf26162

Swift format

6b26145

swift-format 2

a4fde79

Merge branch 'main' into ff-structured-ServerChannel-bind

2d54244

Add ClientBootstrap methods.

b6563f5

ClientBootstrap structured as well

91c1ae3

	public static func address(_ address: SocketAddress) -> BindTarget {
	public static func socketAddress(_ socketAddress: SocketAddress) -> BindTarget {

Add structured concurrency aware ServerBootstrap bind #3378

Are you sure you want to change the base?

Add structured concurrency aware ServerBootstrap bind #3378

Conversation

fabianfett commented Sep 22, 2025

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fabianfett Sep 23, 2025 • 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!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fabianfett Sep 23, 2025 •

edited

Loading