Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ServerBootstrap: should we add the AcceptHandler before or after running the serverChannelInitializer #1392

Closed
weissi opened this issue Feb 13, 2020 · 2 comments
Labels

Comments

@weissi
Copy link
Member

@weissi weissi commented Feb 13, 2020

question: ServerBootstrap: should we add the AcceptHandler before or after running the serverChannelInitializer

CC @normanmaurer

Historically, in NIO we added the AcceptHandler after the serverChannelInitializer. In #1372 I accidentally flipped it around which made a test in swift-nio-extras fail. I'm now changing that back to retain the old behaviour. Nevertheless, we should decide what's right.

@weissi weissi added the discussion label Feb 13, 2020
weissi added a commit to weissi/swift-nio that referenced this issue Feb 13, 2020
Motivation:

For compatibility with 2.13.1 and earlier, we're adding the
AcceptHandler after the serverChannelInitializer runs. If that's the
best solution is discussed in apple#1392.

Modifications:

- add AcceptHandler after the serverChannelInitializer
- give the AcceptHandler the name AcceptHandler

Result:

- better compatibility
weissi added a commit to weissi/swift-nio that referenced this issue Feb 13, 2020
Motivation:

For compatibility with 2.13.1 and earlier, we're adding the
AcceptHandler after the serverChannelInitializer runs. If that's the
best solution is discussed in apple#1392.

Modifications:

- add AcceptHandler after the serverChannelInitializer
- give the AcceptHandler the name AcceptHandler

Result:

- better compatibility
weissi added a commit that referenced this issue Feb 13, 2020
Motivation:

For compatibility with 2.13.1 and earlier, we're adding the
AcceptHandler after the serverChannelInitializer runs. If that's the
best solution is discussed in #1392.

Modifications:

- add AcceptHandler after the serverChannelInitializer
- give the AcceptHandler the name AcceptHandler

Result:

- better compatibility
@normanmaurer

This comment has been minimized.

Copy link
Member

@normanmaurer normanmaurer commented Feb 13, 2020

@weissi in netty we add it after we run the initialiser. This worked great for us and also ensure the user does not mess up the pipeline in a way that would break the accept logic.

@weissi

This comment has been minimized.

Copy link
Member Author

@weissi weissi commented Feb 13, 2020

thanks @normanmaurer , let's match Netty here then and leave it the way it is now on master (and also in all versions ever released).

@weissi weissi closed this Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.