spark-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [spark] vanzin commented on a change in pull request #26609: [SPARK-29971] Fix multiple possible buffer leaks in `TransportFrameDe…
Date Wed, 20 Nov 2019 17:09:46 GMT
vanzin commented on a change in pull request #26609: [SPARK-29971] Fix multiple possible buffer
leaks in `TransportFrameDe…
URL: https://github.com/apache/spark/pull/26609#discussion_r348633255
 
 

 ##########
 File path: common/network-common/src/main/java/org/apache/spark/network/util/ByteArrayReadableChannel.java
 ##########
 @@ -19,23 +19,64 @@
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.nio.channels.ClosedChannelException;
 import java.nio.channels.ReadableByteChannel;
 
 import io.netty.buffer.ByteBuf;
 
 public class ByteArrayReadableChannel implements ReadableByteChannel {
   private ByteBuf data;
+  private boolean closed;
 
   public int readableBytes() {
-    return data.readableBytes();
+    return data == null ? 0 : data.readableBytes();
   }
 
   public void feedData(ByteBuf buf) {
-    data = buf;
+    if (closed) {
+      buf.release();
+      return;
+    }
+    ByteBuf currentData = data;
+    if (currentData == null) {
+      data = buf;
+    } else {
+      int currentReadable = currentData.readableBytes();
 
 Review comment:
   It seems you're making this change so that it's safe to call `feedData` when there is still
data to process.
   
   That's unnecessary; this class is not general purpose, and is only used in one place, where
the surrounding code makes sure all this data is consumed after `feedData` is called. If you're
worried, a check for the buffer being empty + throwing an exception should be enough.
   
   (I'd kinda like to rewrite this stuff at some point to avoid so much copying and add new
features, but that requires time...)

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Mime
View raw message