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

Force flush of nio buffer when threshold is reached #84

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eperott
Copy link

@eperott eperott commented Mar 31, 2020

Update unit tests to verify the FormattedByteChannel with the Netty ChunkedNioStream.

Closes #83

Update unit tests to verify the FormattedByteChannel with the
Netty ChunkedNioStream.

Closes instaclustr#83
if (!isOpen()) {
return -1;
}

// Forcing the calling ChunkedNioStream to flush the buffer
if (hasBufferReachedChunkThreshold(dst)) {
return -1;
Copy link
Author

Choose a reason for hiding this comment

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

The strategy used here is somewhat questionable.

Returning -1 while there's still data to read isn't exactly suggested by the ReadableByteChannel API. On the other hand, the ChunkedNioStream class, which is consuming this byte channel in Netty, seem to expect this behavior deliberately. I've verified this to work on both Netty 4.0 and 4.1.

This was the most elegant solution I could come up with that will avoid another staging buffer, and at the same time stay compatible with both Netty 4.0 and 4.1.

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.

Buffer overflow in nio exposition
1 participant