Darrel Schneider created GEODE-6661:
---------------------------------------
Summary: NioSslEngine has some problems in it ByteBuffer management
Key: GEODE-6661
URL: https://issues.apache.org/jira/browse/GEODE-6661
Project: Geode
Issue Type: Bug
Components: messaging
Reporter: Darrel Schneider
the NioSslEngine appears to have some problems with how it manages ByteBuffer
instances,
# It has a "handshakeBuffer" instance variable and code that will
conditionally create it but higher level code always passes in a non-null
inputBuffer. It should just be changed to require an outside buffer. Also no
need for an instance variable since "handshakeBuffer" is only used by a single
method. It can just be passed in to it.
# The "myNetData" and "peerAppData" are both created as heap ByteBuffer
instances in the constructor. But if they ever need to be resized it does it by
calling Buffers.expandWriteBufferIfNeeded which will return the original heap
ByteBuffer to the queue of buffers that should always be direct byte buffers.
And now the one used by NioSslEngine will be direct instead of heap. This will
also cause the stats that Buffers has to be wrong because we return a buffer to
it that we did not allocate from it.
# From a performance standpoint, we want to also have the buffer that we
directly write to a socket channel, or fill by reading from a socket channel,
be a direct byte buffer. Other byte buffers should not be direct. So normally
the ByteBuffer we serialize an outgoing message into is a direct ByteBuffer
because it will be handed to the socket channel for the message write. But in
the case of the NioSslEngine we would want that first buffer to be a
non-direct, heap ByteBuffer. It ends up being passed to NioSslEngine.wrap which
copies it into "myNetData". The encrypted data in "myNetData" in turn is
written to the socket channel so it is the one that should be a direct
ByteBuffer. For reading it is just the opposite. We read encrypted data from
the socket channel into what should be direct byte buffer (and it currently is
because it is allocated with Buffers). But then it is passed to
NioSslEngine.unwrap which will copy (and decrypt) what is in it into the
"peerAppData". So "peerAppData" should be kept a heap ByteBuffer. You can
always get away with using either type of ByteBuffer. It is simply a
performance issue. What happens at a lower level in the jdk with a heap
ByteBuffer being used with a socket channel is that it eventually just copies
it into a direct ByteBuffer and then uses it. That extra copy can hurt
performance and in the past we had trouble with the jdk caching of direct
ByteBuffers not reusing as well as ours and running out of direct memory.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
