[
https://issues.apache.org/jira/browse/GEODE-6661?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Bruce J Schuchardt reopened GEODE-6661:
---------------------------------------
Assignee: Bruce J Schuchardt (was: Mario Ivanac)
Handshake buffer changes need to be reverted as the handshake buffer may
contain encrypted application data written by the peer. This caused failures
in Benchmark CI tasks (e.g.,
[https://concourse.apachegeode-ci.info/teams/main/pipelines/apache-develop-main/jobs/Benchmark/builds/666]).
I'm preparing a small correction to fix the problem.
> 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
> 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)
