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

SslHandler promise completion incorrect if write doesn't immediately #7380

Closed
wants to merge 1 commit into
base: 4.1
from

Conversation

Projects
None yet
3 participants
@Scottmitch
Member

Scottmitch commented Nov 7, 2017

complete

Motivation:
SslHandler removes a Buffer/Promise pair from
AbstractCoalescingBufferQueue when wrapping data. However it is possible
the SSLEngine will not consume the entire buffer. In this case
SslHandler adds the Buffer back to the queue, but doesn't add the
Promise back to the queue. This may result in the promise completing
immediately in finishFlush, and generally not correlating to the
completion of writing the corresponding Buffer

Modifications:

  • AbstractCoalescingBufferQueue#addFirst should also support adding the
    ChannelPromise
  • In the event of a handshake timeout we should immediately fail pending
    writes immediately to get a more accurate exception

Result:
Fixes #7378.

SslHandler promise completion incorrect if write doesn't immediately
complete

Motivation:
SslHandler removes a Buffer/Promise pair from
AbstractCoalescingBufferQueue when wrapping data. However it is possible
the SSLEngine will not consume the entire buffer. In this case
SslHandler adds the Buffer back to the queue, but doesn't add the
Promise back to the queue. This may result in the promise completing
immediately in finishFlush, and generally not correlating to the
completion of writing the corresponding Buffer

Modifications:
- AbstractCoalescingBufferQueue#addFirst should also support adding the
ChannelPromise
- In the event of a handshake timeout we should immediately fail pending
writes immediately to get a more accurate exception

Result:
Fixes #7378.

@Scottmitch Scottmitch added the defect label Nov 7, 2017

@Scottmitch Scottmitch added this to the 4.1.17.Final milestone Nov 7, 2017

@Scottmitch Scottmitch self-assigned this Nov 7, 2017

@Scottmitch Scottmitch requested a review from normanmaurer Nov 7, 2017

@Scottmitch

This comment has been minimized.

Show comment
Hide comment
@Scottmitch

Scottmitch Nov 7, 2017

Member

@s-gheldd - FYI. Thanks for the reproducer!

Member

Scottmitch commented Nov 7, 2017

@s-gheldd - FYI. Thanks for the reproducer!

@s-gheldd

This comment has been minimized.

Show comment
Hide comment
@s-gheldd

s-gheldd Nov 7, 2017

Contributor

Thanks for the extremely quick reaction.

Contributor

s-gheldd commented Nov 7, 2017

Thanks for the extremely quick reaction.

@Scottmitch

This comment has been minimized.

Show comment
Hide comment
@Scottmitch
Member

Scottmitch commented Nov 7, 2017

4.1 (8c5eeb5)

@Scottmitch Scottmitch closed this Nov 7, 2017

@Scottmitch Scottmitch deleted the Scottmitch:ssl_handler_fail_writes branch Nov 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment