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

Provide ChannelPipeline.remove methods with promises. #651

Merged
merged 1 commit into from Nov 7, 2018

Conversation

Projects
None yet
3 participants
@Lukasa
Contributor

Lukasa commented Nov 7, 2018

Motivation:

ChannelPipeline.remove0 contains a comment that indicates that users
were expected to be able to use the ChannelHandlerContext when both
handlerRemoved and the promise for channel removal call out.

This works when invoking remove() from outside the event loop, but if
a handler invokes remove on itself then it will only be able to attach
a callback to the future after the callout occurs. This means that a
ChannelHandler removing itself from the pipeline cannot rely on there
being a moment when it can still invoke the ChannelHandlerContext, but
when it is no longer a formal part of the pipeline.

This kind of behaviour is useful in some unusual cases, and it would
be nice to support it.

Modifications:

  • Added remove() methods that accept promises as input.
  • Rewrote the current remove() methods in terms of the new ones.
  • Added tests.

Result:

ChannelHandlers can perform cleanup with a valid ChannelHandlerContext
but outside the Channel.

Provide ChannelPipeline.remove methods with promises.
Motivation:

ChannelPipeline.remove0 contains a comment that indicates that users
were expected to be able to use the ChannelHandlerContext when both
handlerRemoved and the promise for channel removal call out.

This works when invoking remove() from outside the event loop, but if
a handler invokes remove on itself then it will only be able to attach
a callback to the future *after* the callout occurs. This means that a
ChannelHandler removing itself from the pipeline cannot rely on there
being a moment when it can still invoke the ChannelHandlerContext, but
when it is no longer a formal part of the pipeline.

This kind of behaviour is useful in some unusual cases, and it would
be nice to support it.

Modifications:

- Added remove() methods that accept promises as input.
- Rewrote the current remove() methods in terms of the new ones.
- Added tests.

Result:

ChannelHandlers can perform cleanup with a valid ChannelHandlerContext
but outside the Channel.

@Lukasa Lukasa added this to the 1.12.0 milestone Nov 7, 2018

@Lukasa Lukasa requested review from normanmaurer and weissi Nov 7, 2018

@weissi

weissi approved these changes Nov 7, 2018

oh wow, surprised that we didn't have those guys yet :).

LGTM!

@Lukasa Lukasa merged commit a0b3b19 into apple:master Nov 7, 2018

3 checks passed

pull request validation (4.0.3) Build finished.
Details
pull request validation (4.1) Build finished.
Details
pull request validation (4.2) Build finished.
Details

@Lukasa Lukasa deleted the Lukasa:cb-passing-promises-is-better branch Nov 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment