andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix write watermarks comparison to use less than and greater than.

Browse Source 
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:
buchgr 2016-08-24 10:56:21 +02:00 committed by Norman Maurer
parent 126965aac6
commit d79e5c594d
2 changed files with 8 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
transport/src/main/java/io/netty/channel/ChannelOutboundBuffer.java
View File

@ -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);
}
}

10
transport/src/test/java/io/netty/channel/ChannelOutboundBufferTest.java
View File

@ -220,19 +220,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);
@ -329,7 +331,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()