Description
Hello again. I promise this is the last bug report for today :)
Another thing I noticed in my snooping is this piece of code.
According to SocketChannel.write() docs:
Some types of channels, depending upon their state, may write only some of the bytes or possibly none at all. A socket channel in non-blocking mode, for example, cannot write any more bytes than are free in the socket's output buffer.
If I'm understanding correctly, the socket's output buffer size is dependent on the SO_SNDBUF socket option, whose default value will be system dependent.
If a TCP message is big enough, it can exceed this buffer, and thus cause the linked code in the library to fail and a partial message to be sent.
I see this was changed relatively recently in this commit titled "Extend logging in NIO clients to trace bugs". This is however a functional change. Futhermore that while(buffer.hasRemaining()) loop seems basically useless in its current form, as we fail in all cases in which we couldn't write the whole buffer in one go, so the body of the while would never be executed more than once.
My question is:
- Was the functional change in that commit intended, or was it an oversight while adding logging? If it was intended, what was the reason?
- Should that piece of code be rolled-back to what it was before (so fail only if
n < 0)? In my experiments doing that fixed the issue we were having when sending messages bigger the the output buffer.
Also to note: that method is called in the selector thread. I'm no expert on this, but calling write() again just after filling the buffer I believe would be blocking, which the selector thread shouldn't do. I'm not sure however what a "proper" fix would look like, but I'd rather take the performance hit of the selector thread being blocked rather than hard-failing and sending a partial TCP message.