Skip to content

Conversation

Lukasa
Copy link
Contributor

@Lukasa Lukasa commented Jul 17, 2025

Motivation:

For our async initializers on bootstraps, it's imortant that both channel initializers are called, not only the one that comes as an argument on the bind function.

Modifications:

Fix the takeOwnershipOfDescriptor async functions to call the initializer from the bootstrap, and add regression tests.

Result:

Better behaved code.

Motivation:

For our async initializers on bootstraps, it's imortant that _both_
channel initializers are called, not only the one that comes as an
argument on the bind function.

Modifications:

Fix the takeOwnershipOfDescriptor async functions to call the initializer
from the bootstrap, and add regression tests.

Result:

Better behaved code.
@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Jul 17, 2025
Copy link
Contributor

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops! Nice catch.

@glbrntt glbrntt merged commit ee67a96 into apple:main Jul 18, 2025
41 checks passed
zaneenders pushed a commit to zaneenders/swift-nio that referenced this pull request Jul 23, 2025
…pple#3309)

Motivation:

For our async initializers on bootstraps, it's imortant that _both_
channel initializers are called, not only the one that comes as an
argument on the bind function.

Modifications:

Fix the takeOwnershipOfDescriptor async functions to call the
initializer from the bootstrap, and add regression tests.

Result:

Better behaved code.
@Austinpayne
Copy link
Contributor

@Lukasa I'm hitting a precondition failure after bumping from nio 2.84.0 to 2.85.0 and I believe it is due to this change.

I'm adding a ByteToMessageHandler in .channelInitializer then calling .takingOwnershipOfDescriptor (the event loop based API, not the async one) and it hits this precondition in Codec.swift. I believe this is due to the path through .takingOwnershipOfDescriptor calling the channel initializer twice, once here and the second time here (which for my code path, calls self.channelInitializer twice).

I'm happy to submit a reproducer but curious on your thoughts first and what the fix would be. I might be misusing the API to begin with. I'm able to work around it by initializing the channel after taking ownership:

Broken (hits precondition):
Screenshot 2025-07-31 at 3 23 02 PM

Work around:
Screenshot 2025-07-31 at 3 23 22 PM

@Lukasa
Copy link
Contributor Author

Lukasa commented Aug 1, 2025

Ah, yeah, I see the issue. This new API surface is frustratingly under-tested.

Lukasa added a commit to Lukasa/swift-nio that referenced this pull request Aug 1, 2025
Motivation:

In apple#3309 I fixed an issue where the channel initializer would not be
called in some PipeBootstrap paths. Unfortunately, that introduced
a new bug where the initializer was now called twice on some other
paths.

Modifications:

Get the number of calls back to 1 on the non-async paths.
Add some tests

Result:

Better pipe bootstrap behaviour.
@Lukasa
Copy link
Contributor Author

Lukasa commented Aug 1, 2025

See #3330.

Lukasa added a commit that referenced this pull request Aug 1, 2025
Motivation:

In #3309 I fixed an issue where the channel initializer would not be
called in some PipeBootstrap paths. Unfortunately, that introduced a new
bug where the initializer was now called twice on some other paths.

Modifications:

Get the number of calls back to 1 on the non-async paths. Add some tests

Result:

Better pipe bootstrap behaviour.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants