DefaultHttp2RemoteFlowController reentry infinite loop

Motivation:
DefaultHttp2RemoteFlowController.writePendingBytes does not support reentry but does not enforce this constraint. Reentry is possible if the channel transitions from Writable -> Not Writable -> Writable during the distribution phase. This can happen if the user flushes when notified of the channel transitioning to the not writable state, and may be done if the user wants to fill the channel outbound buffer before flushing.

Modifications:
- DefaultHttp2RemoteFlowController.writePendingBytes should protect against reentry

Result:
DefaultHttp2RemoteFlowController will not allocate unexpected amounts or enter an infinite loop.
This commit is contained in:
Scott Mitchell 2016-07-08 11:59:21 -07:00
parent 77770374fb
commit 9de90d07a9

View File

@ -542,6 +542,7 @@ public class DefaultHttp2RemoteFlowController implements Http2RemoteFlowControll
* Abstract class which provides common functionality for writability monitor implementations. * Abstract class which provides common functionality for writability monitor implementations.
*/ */
private class WritabilityMonitor { private class WritabilityMonitor {
private boolean inWritePendingBytes;
private long totalPendingBytes; private long totalPendingBytes;
private final Writer writer = new StreamByteDistributor.Writer() { private final Writer writer = new StreamByteDistributor.Writer() {
@Override @Override
@ -613,16 +614,28 @@ public class DefaultHttp2RemoteFlowController implements Http2RemoteFlowControll
} }
final void writePendingBytes() throws Http2Exception { final void writePendingBytes() throws Http2Exception {
int bytesToWrite = writableBytes(); // Reentry is not permitted during the byte distribution process. It may lead to undesirable distribution of
// bytes and even infinite loops. We protect against reentry and make sure each call has an opportunity to
// Make sure we always write at least once, regardless if we have bytesToWrite or not. // cause a distribution to occur. This may be useful for example if the channel's writability changes from
// This ensures that zero-length frames will always be written. // Writable -> Not Writable (because we are writing) -> Writable (because the user flushed to make more room
for (;;) { // in the channel outbound buffer).
if (!streamByteDistributor.distribute(bytesToWrite, writer) || if (inWritePendingBytes) {
(bytesToWrite = writableBytes()) <= 0 || return;
!isChannelWritable0()) { }
break; inWritePendingBytes = true;
try {
int bytesToWrite = writableBytes();
// Make sure we always write at least once, regardless if we have bytesToWrite or not.
// This ensures that zero-length frames will always be written.
for (;;) {
if (!streamByteDistributor.distribute(bytesToWrite, writer) ||
(bytesToWrite = writableBytes()) <= 0 ||
!isChannelWritable0()) {
break;
}
} }
} finally {
inWritePendingBytes = false;
} }
} }