Skip to content

fix remove handler cause ByteToMessageDecoder out disorder#9670

Merged
normanmaurer merged 5 commits intonetty:4.1from
switchYello:4.1
Oct 21, 2019
Merged

fix remove handler cause ByteToMessageDecoder out disorder#9670
normanmaurer merged 5 commits intonetty:4.1from
switchYello:4.1

Conversation

@switchYello
Copy link
Copy Markdown
Contributor

@switchYello 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 .

@netty-bot
Copy link
Copy Markdown

netty-bot commented Oct 15, 2019

Can one of the admins verify this patch?

normanmaurer
normanmaurer requested changes Oct 15, 2019
View reviewed changes
@normanmaurer
Copy link
Copy Markdown
Member

normanmaurer commented Oct 15, 2019

@switchYello ping me once you addressed the comments

@switchYello
Copy link
Copy Markdown
Contributor Author

switchYello commented Oct 16, 2019

@normanmaurer yes

@normanmaurer
Copy link
Copy Markdown
Member

normanmaurer commented Oct 16, 2019

@johnou PTAL

@normanmaurer
Copy link
Copy Markdown
Member

normanmaurer commented Oct 16, 2019

@netty-bot test this please

@johnou
Copy link
Copy Markdown
Contributor

johnou commented Oct 16, 2019

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

@normanmaurer
Copy link
Copy Markdown
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.

@normanmaurer
Copy link
Copy Markdown
Member

normanmaurer commented Oct 17, 2019

@netty-bot test this please

@switchYello
Copy link
Copy Markdown
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
Copy link
Copy Markdown
Contributor Author

switchYello commented Oct 17, 2019

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

@normanmaurer
Copy link
Copy Markdown
Member

normanmaurer commented Oct 17, 2019

@netty-bot test this please

@normanmaurer
Copy link
Copy Markdown
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
normanmaurer
normanmaurer requested changes Oct 17, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@normanmaurer normanmaurer left a comment

Choose a reason for hiding this comment

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

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

johnou
johnou approved these changes Oct 17, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@johnou johnou left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@normanmaurer nvm, lgtm!

@switchYello
Copy link
Copy Markdown
Contributor Author

switchYello commented Oct 18, 2019 •
edited
Loading

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

@normanmaurer
Copy link
Copy Markdown
Member

normanmaurer commented Oct 19, 2019

@netty-bot test this please

@normanmaurer normanmaurer merged commit e745ef0 into netty:4.1 Oct 21, 2019
@normanmaurer
Copy link
Copy Markdown
Member

normanmaurer commented Oct 21, 2019

@switchYello thanks a lot!

normanmaurer pushed a commit that referenced this pull request Oct 21, 2019
@switchYello @normanmaurer
fix remove handler cause ByteToMessageDecoder out disorder (#9670)
402537c 
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

Reviewers

@normanmaurer normanmaurer normanmaurer approved these changes

+1 more reviewer

@johnou johnou johnou approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.1.43.Final

Development

Successfully merging this pull request may close these issues.

remove handler cause ByteToMessageDecoder out disorder

4 participants

@switchYello @netty-bot @normanmaurer @johnou