Skip to content

Bug in BufferedWriteHandler fixed#618

Merged
normanmaurer merged 1 commit into
netty:3from
stefanmk:patch-2
Sep 19, 2012
Merged

Bug in BufferedWriteHandler fixed#618
normanmaurer merged 1 commit into
netty:3from
stefanmk:patch-2

Conversation

@stefanmk
Copy link
Copy Markdown

flush variable in BufferedWriteHandler was never reset. This caused the buffer to only be flushed once and then never again. Bug was introduced in commit 88124d8. Without this fix BufferedWriteHandler is completely broken and useless as it doesn't work at all.

…er.java

fix bug, flush variable was never reset
@normanmaurer
Copy link
Copy Markdown
Member

@stefanmk didn't you also have a unit test for this ?

@stefanmk
Copy link
Copy Markdown
Author

No, sorry, I do not have a unit test.

We are using Netty in a server project, 3.5.7. complety broke it. With the fix all is working fine again.

If you take a closer look at the code it is clear that the code cannot work and that there must be a bug. The flush variable is never set to false again, so after the first flush there cannot be a second one.

@normanmaurer
Copy link
Copy Markdown
Member

@stefanmk I thought I saw an unit test in the other pull-request that got closed.. anyway your fix looks good :)

normanmaurer added a commit that referenced this pull request Sep 19, 2012
Bug in BufferedWriteHandler fixed
@normanmaurer normanmaurer merged commit a7d2050 into netty:3 Sep 19, 2012
@ghost ghost assigned normanmaurer Sep 19, 2012
@normanmaurer normanmaurer mentioned this pull request Apr 1, 2013
chrisvest pushed a commit to chrisvest/netty that referenced this pull request Apr 2, 2025
Motivation:

How we used QuicConnectEvent to notify about connection migration was
not correct at all. Let's remove it for now as we will introduce
something better once cloudflare/quiche#1660 was
merged into quiche.

Modifications:

Remove QuicConnectEvent

Result:

Get rid of incorrect implementation to prepare for adding a proper one
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants