[
https://issues.apache.org/jira/browse/GEODE-6661?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16968791#comment-16968791
]
ASF subversion and git services commented on GEODE-6661:
--------------------------------------------------------
Commit 1ffe53b9eb53f2d55cf1c3ef681d8545b54116b3 in geode's branch
refs/heads/release/1.11.0 from Bruce Schuchardt
[ https://gitbox.apache.org/repos/asf?p=geode.git;h=1ffe53b ]
Feature/geode 6661 (#4284)
* GEODE-6661 NioSslEngine has some problems in its ByteBuffer management
Reverting the change to use a temporary byte buffer for SSL handshakes.
At the end of a handshake the buffer may contain application data that
must be available for subsequent decryption. In the case of TCPConduit
this is usually the "handshake" bytes transmitted for that package's
communications protocol.
Since we really need those bytes I've removed the option of expanding
the handshake buffer if it's smaller than the SSL session's required
packet size. TCPConduit uses that figure to allocate the buffer so this
should be safe. I've added a test for this.
* reverting investigative changes to test
* fix failing unit tests - adjusted buffer sizes
* reverted another set of investigative test changes
> NioSslEngine has some problems in its ByteBuffer management
> -----------------------------------------------------------
>
> Key: GEODE-6661
> URL: https://issues.apache.org/jira/browse/GEODE-6661
> Project: Geode
> Issue Type: Bug
> Components: messaging
> Reporter: Darrel Schneider
> Assignee: Bruce J Schuchardt
> Priority: Major
> Labels: performance
> Fix For: 1.12.0
>
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> 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
(v8.3.4#803005)
