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

Ensure channel handler close() is not skipped in !hasDisconnect case #9098

Merged
merged 1 commit into from Apr 28, 2019

Conversation

Projects
None yet
4 participants
@njhill
Copy link
Member

commented Apr 27, 2019

Motivation

The optimization in #8988 didn't correctly handle the specific case where the channel hasDisconnect == false, and a ChannelOutboundHandlerAdapter subclass overrides only the close(ctx, promise) method without also overriding the disconnect(ctx, promise) method.

Modifications

Adjust AbstractChannelHandler.disconnect(...) method to divert to close(...) in !hasDisconnect case before computing target context for the event.

Result

Fixes #9092

Ensure channel handler close() is not skipped in !hasDisconnect case
Motivation

The optimization in #8988 didn't correctly handle the specific case
where the channel hasDisconnect == false, and a
ChannelOutboundHandlerAdapter subclass overrides only the close(ctx,
promise) method without also overriding the disconnect(ctx, promise)
method.

Modifications

Adjust AbstractChannelHandler.disconnect(...) method to divert to
close(...) in !hasDisconnect case before computing target context for
the event.

Result

Fixes #9092

@njhill njhill added the defect label Apr 27, 2019

@njhill njhill requested review from daschl and normanmaurer Apr 27, 2019

@netty-bot

This comment has been minimized.

Copy link

commented Apr 27, 2019

Can one of the admins verify this patch?

@normanmaurer

This comment has been minimized.

Copy link
Member

commented Apr 27, 2019

@netty-bot test this please

@normanmaurer normanmaurer merged commit 00a9a25 into netty:4.1 Apr 28, 2019

3 checks passed

pull request validation (centos6-java11) Build finished.
Details
pull request validation (centos6-java12) Build finished.
Details
pull request validation (centos6-java8) Build finished.
Details
@normanmaurer

This comment has been minimized.

Copy link
Member

commented Apr 28, 2019

@njhill thanks a lot!

@normanmaurer normanmaurer added this to the 4.1.36.Final milestone Apr 28, 2019

normanmaurer added a commit that referenced this pull request Apr 28, 2019

Ensure channel handler close() is not skipped in !hasDisconnect case (#…
…9098)

Motivation

The optimization in #8988 didn't correctly handle the specific case
where the channel hasDisconnect == false, and a
ChannelOutboundHandlerAdapter subclass overrides only the close(ctx,
promise) method without also overriding the disconnect(ctx, promise)
method.

Modifications

Adjust AbstractChannelHandler.disconnect(...) method to divert to
close(...) in !hasDisconnect case before computing target context for
the event.

Result

Fixes #9092

@njhill njhill deleted the njhill:fix-disconnect branch Apr 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.