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

fix remove handler cause ByteToMessageDecoder out disorder #9670

Merged
merged 5 commits into from Oct 21, 2019

Conversation

@switchYello
Copy link
Contributor

switchYello commented Oct 15, 2019

Motivation:

Data flowing in from the decoder flows out in sequence,Whether decoder removed or not.

Modification:

fire data in out and clear out when hander removed
before call method handlerRemoved(ctx)

Result:

Fixes #9668 .

@switchYello

This comment has been minimized.

fire data in out and clear out when hander removed
before call method handlerRemoved(ctx)

@netty-bot

This comment has been minimized.

Copy link

netty-bot commented Oct 15, 2019

Can one of the admins verify this patch?

@normanmaurer

This comment has been minimized.

Copy link
Member

normanmaurer commented Oct 15, 2019

@switchYello ping me once you addressed the comments

switchYello
@switchYello

This comment has been minimized.

Copy link
Contributor Author

switchYello commented Oct 16, 2019

@normanmaurer

This comment has been minimized.

Copy link
Member

normanmaurer commented Oct 16, 2019

@johnou PTAL

@normanmaurer

This comment has been minimized.

Copy link
Member

normanmaurer commented Oct 16, 2019

@netty-bot test this please

@johnou

This comment has been minimized.

Copy link
Contributor

johnou commented Oct 16, 2019

@normanmaurer what about #4635 ? or do you see this as another edge case?

@normanmaurer

This comment has been minimized.

Copy link
Member

normanmaurer commented Oct 16, 2019

@switchYello please fix the check style errors:

15:17:12 [INFO] Starting audit...
15:17:12 [ERROR] /code/codec/src/test/java/io/netty/handler/codec/ByteToMessageDecoderTest.java:404:31: '{' is not preceded with whitespace. [WhitespaceAround]
15:17:12 [ERROR] /code/codec/src/test/java/io/netty/handler/codec/ByteToMessageDecoderTest.java:406:17: Variable 'count' explicitly initialized to '0' (default value for its type). [ExplicitInitialization]
15:17:12 [ERROR] /code/codec/src/test/java/io/netty/handler/codec/ByteToMessageDecoderTest.java:419:27: 'typecast' is not followed by whitespace. [WhitespaceAfter]
15:17:12 [ERROR] /code/codec/src/test/java/io/netty/handler/codec/ByteToMessageDecoderTest.java:420:27: 'typecast' is not followed by whitespace. [WhitespaceAfter]
15:17:12 [ERROR] /code/codec/src/test/java/io/netty/handler/codec/ByteToMessageDecoderTest.java:421:27: 'typecast' is not followed by whitespace. [WhitespaceAfter]
15:17:12 [ERROR] /code/codec/src/test/java/io/netty/handler/codec/ByteToMessageDecoderTest.java:422:27: 'typecast' is not followed by whitespace. [WhitespaceAfter]
15:17:12 [ERROR] /code/codec/src/test/java/io/netty/handler/codec/ByteToMessageDecoderTest.java:424:27: 'typecast' is not followed by whitespace. [WhitespaceAfter]
15:17:12 Audit done.
switchYello
@normanmaurer

This comment has been minimized.

Copy link
Member

normanmaurer commented Oct 17, 2019

@netty-bot test this please

@switchYello

This comment has been minimized.

Copy link
Contributor Author

switchYello commented Oct 17, 2019

@normanmaurer

the output must be byte,i conversion to byte fail,I don't understand

12:07:22 [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.0:testCompile (default-testCompile) on project netty-codec: Compilation failure: Compilation failure: 
12:07:22 [ERROR] /code/codec/src/test/java/io/netty/handler/codec/ByteToMessageDecoderTest.java:[419,57] error: incompatible types: Object cannot be converted to byte
12:07:22 [ERROR] /code/codec/src/test/java/io/netty/handler/codec/ByteToMessageDecoderTest.java:[420,57] error: incompatible types: Object cannot be converted to byte
12:07:22 [ERROR] /code/codec/src/test/java/io/netty/handler/codec/ByteToMessageDecoderTest.java:[421,57] error: incompatible types: Object cannot be converted to byte
12:07:22 [ERROR] /code/codec/src/test/java/io/netty/handler/codec/ByteToMessageDecoderTest.java:[422,57] error: incompatible types: Object cannot be converted to byte

if i not conversion ,my idea will prompt me

ideal

and now I don't know how to do it. remove type conversion ?

switchYello
@switchYello

This comment has been minimized.

Copy link
Contributor Author

switchYello commented Oct 17, 2019

oh i konw ,because my languagel level is 1.8 ... ,netty need 1.6

@normanmaurer

This comment has been minimized.

Copy link
Member

normanmaurer commented Oct 17, 2019

@netty-bot test this please

@normanmaurer

This comment has been minimized.

Copy link
Member

normanmaurer commented Oct 17, 2019

@johnou sorry I don't get what you are asking for... Can you elaborate ?

@normanmaurer normanmaurer added this to the 4.1.43.Final milestone Oct 17, 2019
Copy link
Member

normanmaurer left a comment

one last nit then ready to go... great catch

@johnou
johnou approved these changes Oct 17, 2019
Copy link
Contributor

johnou left a comment

@normanmaurer nvm, lgtm!

@switchYello

This comment has been minimized.

Copy link
Contributor Author

switchYello commented Oct 18, 2019

@normanmaurer yes i add assertFalse(buffer5.isReadable()) ,ensure not produce more then expected

@normanmaurer

This comment has been minimized.

Copy link
Member

normanmaurer commented Oct 19, 2019

@netty-bot test this please

@normanmaurer normanmaurer merged commit e745ef0 into netty:4.1 Oct 21, 2019
3 checks passed
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

normanmaurer commented Oct 21, 2019

@switchYello thanks a lot!

normanmaurer added a commit that referenced this pull request Oct 21, 2019
Motivation:

Data flowing in from the decoder flows out in sequence,Whether decoder removed or not.

Modification:

fire data in out and clear out when hander removed
before call method handlerRemoved(ctx)

Result:

Fixes #9668 .
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.