Fix write watermarks comparison to use less than and greater than.
Motivation: The API documentation in ChannelConfig states that a a channel is writable, if the number of pending bytes is below the low watermark and a channel is not writable, if the number of pending bytes exceeds the high watermark. Therefore, we should use < operators instead of <= as well as > instead of >=. Using <= and >= is also problematic, if the low watermark is equal to the high watermark, as then a channel could be both writable and unwritable with the same number of pending bytes (depending on whether remove() or addMessage() is called first). The use of <= and >= was introduced in PR https://github.com/netty/netty/pull/3036, but I don't understand why, as there doesn't seem to have been any discussion around that. Modifications: Use < and > operators instead of <= and >=. Result: High and low watermarks are treated as stated in the API docs.
This commit is contained in:
parent
a8b8553ad1
commit
4accd300e5
@ -175,7 +175,7 @@ public final class ChannelOutboundBuffer {
|
||||
}
|
||||
|
||||
long newWriteBufferSize = TOTAL_PENDING_SIZE_UPDATER.addAndGet(this, size);
|
||||
if (newWriteBufferSize >= channel.config().getWriteBufferHighWaterMark()) {
|
||||
if (newWriteBufferSize > channel.config().getWriteBufferHighWaterMark()) {
|
||||
setUnwritable(invokeLater);
|
||||
}
|
||||
}
|
||||
@ -194,8 +194,7 @@ public final class ChannelOutboundBuffer {
|
||||
}
|
||||
|
||||
long newWriteBufferSize = TOTAL_PENDING_SIZE_UPDATER.addAndGet(this, -size);
|
||||
if (notifyWritability && (newWriteBufferSize == 0
|
||||
|| newWriteBufferSize <= channel.config().getWriteBufferLowWaterMark())) {
|
||||
if (notifyWritability && newWriteBufferSize < channel.config().getWriteBufferLowWaterMark()) {
|
||||
setWritable(invokeLater);
|
||||
}
|
||||
}
|
||||
|
@ -221,19 +221,21 @@ public class ChannelOutboundBufferTest {
|
||||
ch.config().setWriteBufferLowWaterMark(128);
|
||||
ch.config().setWriteBufferHighWaterMark(256);
|
||||
|
||||
// Ensure exceeding the low watermark does not make channel unwritable.
|
||||
ch.write(buffer().writeZero(128));
|
||||
// Ensure exceeding the low watermark does not make channel unwritable.
|
||||
ch.write(buffer().writeZero(2));
|
||||
assertThat(buf.toString(), is(""));
|
||||
|
||||
ch.unsafe().outboundBuffer().addFlush();
|
||||
|
||||
// Ensure exceeding the high watermark makes channel unwritable.
|
||||
ch.write(buffer().writeZero(128));
|
||||
ch.write(buffer().writeZero(127));
|
||||
assertThat(buf.toString(), is("false "));
|
||||
|
||||
// Ensure going down to the low watermark makes channel writable again by flushing the first write.
|
||||
assertThat(ch.unsafe().outboundBuffer().remove(), is(true));
|
||||
assertThat(ch.unsafe().outboundBuffer().totalPendingWriteBytes(), is(128L));
|
||||
assertThat(ch.unsafe().outboundBuffer().remove(), is(true));
|
||||
assertThat(ch.unsafe().outboundBuffer().totalPendingWriteBytes(), is(127L));
|
||||
assertThat(buf.toString(), is("false true "));
|
||||
|
||||
safeClose(ch);
|
||||
@ -330,7 +332,7 @@ public class ChannelOutboundBufferTest {
|
||||
ChannelOutboundBuffer cob = ch.unsafe().outboundBuffer();
|
||||
|
||||
// Trigger channelWritabilityChanged() by writing a lot.
|
||||
ch.write(buffer().writeZero(256));
|
||||
ch.write(buffer().writeZero(257));
|
||||
assertThat(buf.toString(), is("false "));
|
||||
|
||||
// Ensure that setting a user-defined writability flag to false does not trigger channelWritabilityChanged()
|
||||
|
Loading…
Reference in New Issue
Block a user