normanmaurer commented on a change in pull request #26609: [SPARK-29971] Fix
multiple possible buffer leaks in `TransportFrameDecoder/TransportCipher`
URL: https://github.com/apache/spark/pull/26609#discussion_r348703673
##########
File path:
common/network-common/src/main/java/org/apache/spark/network/util/ByteArrayReadableChannel.java
##########
@@ -45,18 +86,24 @@ public int read(ByteBuffer dst) throws IOException {
if (data.readableBytes() == 0) {
data.release();
+ data = null;
}
return totalRead;
}
@Override
public void close() throws IOException {
+ closed = true;
+ if (data != null) {
+ data.release();
Review comment:
@vanzin again this class is public and so I think it is not guaranteed that
read(...) will always consume the whole buffer. The `ByteArrayReadableChannel`
is wrapped in a `CryptoInputStream` and who knows if it always is guaranteed
that it will consume all data (for example there is at least one case where you
catch an `InternalError` which may cause that the previous feed data was not
released (as it was not fully consumed).
So with all this in mind I think it is the safest to at least ensure we
release everything once `close()` is called.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
