Bill commented on a change in pull request #5666:
URL: https://github.com/apache/geode/pull/5666#discussion_r513655587



##########
File path: 
geode-core/src/main/java/org/apache/geode/internal/net/NioPlainEngine.java
##########
@@ -121,8 +121,12 @@ public void doneReading(ByteBuffer unwrappedBuffer) {
   }
 
   @Override
-  public ByteBuffer getUnwrappedBuffer(ByteBuffer wrappedBuffer) {
-    return wrappedBuffer;
+  public ByteBufferSharing getUnwrappedBuffer() {
+    return shareBuffer(null);

Review comment:
       This suggestion seems intuitive enough, however, where this is called 
from `Connection.readAck()` we rely on the current "no-op" behavior. If we were 
to throw here then it would break `readAck()` using the `NioPlainEngine`.
   
   The only other place we call `getUnwrappedBuffer()` is from 
`clearSSLInputBuffer()` and in that case we know we are dealing with the 
`NioSslEngine` (not the `NioPlainEngine`.) We considered moving this method 
from the interface to the (`NioSslEngine`) implementation (and leaving it off 
`NioPlainEngine` entirely.) That would have been fine for `clearSSLInputBuffer` 
but would have left us with ugliness in the `readAck()` call site.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to