Do various doc and other minor improvements

[stIncMale]

Various things I noticed while working on JAVA-5018.

[jyemin]

Is this true? SocketStream does not seem to do it.

[jyemin]

It looks like ByteBufferBsonOutput#close, called from InternalStreamConnection#sendCommandMessage, is responsible for the release.

[stIncMale]

The apparent deviation from this requirement that you see in SocketStream/AsynchronousChannelStream.write is a result of two peculiarities (I am being generous here):

1. Neither SocketStream.write nor AsynchronousChannelStream.write use the buffers they were given after completing (either returning in the synchronous case, or completing the handler in the asynchronous case). So even if the buffers stay unreleased by write, the method does not leave a reference to them in the heap memory. This, in turn, means that there may neither be negative effects of aliasing, nor memory leaks caused by write not releasing the buffers. However, not releasing may render buffer pooling useless. The latter does not happen due to the next item.
2. SocketStream/AsynchronousChannelStream.write receive the PooledByteBufNIO implementation of ByteBuf backed by ByteBufNIO, or (I am not sure) CompositeByteBuf. All of these ByteBuf implementations implement the duplicate method by creating a new wrapping ByteBuf whose reference count is unrelated to the wrapped buffer: duplicate() does not retain the wrapper, and duplicate().release() does not release it. Such implementations are, of course, a recipe for disaster, because it allows the wrapped buffer to be released despite the wrapping buffer still having positive reference count, which may result in the wrapped buffer being aliased.

TL;DR: SocketStream/AsynchronousChannelStream.write do not release buffers, but releasing the buffers they are given is completely pointless anyway due to how they are implemented. Our code here is brittle and seemingly works only due to a lucky coincidence.

The situation with the NettyByteBuf implementation of ByteBuf and NettyStream is different: NettyByteBuf.duplicate does retain the wrapped buffer and the wrapper delegates its retain/release/getReferenceCount to the wrapped buffer. These two things (retaining when wrapping + delegation) guarantees that the wrapper's lifetime is not larger than the wrapped's lifetime. And NettyStream.write actually releases the buffers: "outbound messages are created by your application, and it is the responsibility of Netty to release these after writing them out to the wire".

So, the documentation change for the Stream.write message is correct, but needs a refinement in light of OutputBuffer.getByteBuffers behaving not the way I expected (I checked how it behaves with NettyByteBuf, but the behavior turned out to be different for other ByteBuf implementations). SocketStream/AsynchronousChannelStream violate it and get away with that only because ByteBufNIO and CompositeByteBuf are also broken. The behavior of ByteBufNIO/CompositeByteBuf.duplicate means that I need to fix the doc change for the OutputBuffer.getByteBuffers method (see the new commit).

[jyemin]

I'm still not so sure. Generally when a reference counted object is passed as a parameter, my assumption is that it's the caller's responsibility to release its own reference to to it when the call completes. The receiving object should only release if it also retains, and it doesn't need to retain unless it keeps it for longer than the completion of the call.

For "async" calls it's a bit different in that the above applies not until the async call returns, but until the callback is invoked.

So to my eyes the bug is that InternalStreamConnection calls ByteBufferBsonOutput#getByteBuffers and receives a list of List<ByteBuf> that it, in come cases, does not release.

[stIncMale]

I rolled back changes in Stream and created https://jira.mongodb.org/browse/JAVA-5084.

[jyemin]

Same as above. AsynchronousChannelStream doesn't seem to do it.

[rozza]

LGTM

[jyemin]

Discussed offline.