Details
-
Bug
-
Status: Closed
-
Major
-
Resolution: Fixed
-
None
Description
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.
Attachments
Issue Links
- causes
-
-
- Closed
-
- links to
