cloud-fan commented on a change in pull request #27064: 
[SPARK-30246]OneForOneStreamManager might leak memory in connectionTerminated
URL: https://github.com/apache/spark/pull/27064#discussion_r366240190
 
 

 ##########
 File path: 
common/network-common/src/main/java/org/apache/spark/network/server/OneForOneStreamManager.java
 ##########
 @@ -117,21 +117,32 @@ public static String genStreamChunkId(long streamId, int 
chunkId) {
 
   @Override
   public void connectionTerminated(Channel channel) {
+    boolean failedToReleaseBuffers = false;
+
     // Close all streams which have been associated with the channel.
     for (Map.Entry<Long, StreamState> entry: streams.entrySet()) {
       StreamState state = entry.getValue();
       if (state.associatedChannel == channel) {
         streams.remove(entry.getKey());
 
-        // Release all remaining buffers.
-        while (state.buffers.hasNext()) {
-          ManagedBuffer buffer = state.buffers.next();
-          if (buffer != null) {
-            buffer.release();
+        try {
+          // Release all remaining buffers.
+          while (state.buffers.hasNext()) {
+            ManagedBuffer buffer = state.buffers.next();
+            if (buffer != null) {
+              buffer.release();
+            }
           }
+        } catch (RuntimeException e) {
+          failedToReleaseBuffers = true;
+          logger.error("Exception trying to release remaining StreamState 
buffers", e);
         }
       }
     }
+
+    if (failedToReleaseBuffers) {
 
 Review comment:
   instead of using a boolean flag, can we keep the actual exception (we can 
just keep the first one) and rethrow it? then we can retain the stack trace.

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


With regards,
Apache Git Services

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

Reply via email to