[hadoop] branch branch-3.3.5 updated: HADOOP-18621. Resource leak in CryptoOutputStream.close() (#5347)

[stevel]

This is an automated email from the ASF dual-hosted git repository.

stevel pushed a commit to branch branch-3.3.5
in repository https://gitbox.apache.org/repos/asf/hadoop.git


The following commit(s) were added to refs/heads/branch-3.3.5 by this push:
     new ff048fce66b HADOOP-18621. Resource leak in CryptoOutputStream.close() 
(#5347)
ff048fce66b is described below

commit ff048fce66b6d06f1e896740423b091596a58309
Author: gardenia <colm.dou...@gmail.com>
AuthorDate: Tue Feb 7 12:01:57 2023 +0000

    HADOOP-18621. Resource leak in CryptoOutputStream.close() (#5347)
    
    When closing we need to wrap the flush() in a try .. finally, otherwise
    when flush throws it will stop completion of the remainder of the
    close activities and in particular the close of the underlying wrapped
    stream object resulting in a resource leak.
    
    Contributed by Colm Dougan
---
 .../org/apache/hadoop/crypto/CryptoOutputStream.java | 13 ++++++++-----
 .../hadoop/crypto/TestCryptoOutputStreamClosing.java | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git 
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoOutputStream.java
 
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoOutputStream.java
index fb5ee21190f..df36bd6fe69 100644
--- 
a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoOutputStream.java
+++ 
b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/CryptoOutputStream.java
@@ -242,12 +242,15 @@ public class CryptoOutputStream extends 
FilterOutputStream implements
       return;
     }
     try {
-      flush();
-      if (closeOutputStream) {
-        super.close();
-        codec.close();
+      try {
+        flush();
+      } finally {
+        if (closeOutputStream) {
+          super.close();
+          codec.close();
+        }
+        freeBuffers();
       }
-      freeBuffers();
     } finally {
       closed = true;
     }
diff --git 
a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/TestCryptoOutputStreamClosing.java
 
b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/TestCryptoOutputStreamClosing.java
index 39e4bb85880..04cdb962ac9 100644
--- 
a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/TestCryptoOutputStreamClosing.java
+++ 
b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/TestCryptoOutputStreamClosing.java
@@ -17,12 +17,14 @@
  */
 package org.apache.hadoop.crypto;
 
+import java.io.IOException;
 import java.io.OutputStream;
 
 import org.apache.hadoop.conf.Configuration;
 
 import org.junit.BeforeClass;
 import org.junit.Test;
+import static org.apache.hadoop.test.LambdaTestUtils.intercept;
 import static org.mockito.Mockito.*;
 
 /**
@@ -54,4 +56,22 @@ public class TestCryptoOutputStreamClosing {
     verify(outputStream, never()).close();
   }
 
+  @Test
+  public void testUnderlyingOutputStreamClosedWhenExceptionClosing() throws 
Exception {
+    OutputStream outputStream = mock(OutputStream.class);
+    CryptoOutputStream cos = spy(new CryptoOutputStream(outputStream, codec,
+        new byte[16], new byte[16], 0L, true));
+
+    // exception while flushing during close
+    doThrow(new IOException("problem flushing wrapped stream"))
+        .when(cos).flush();
+
+    intercept(IOException.class,
+        () -> cos.close());
+
+    // We expect that the close of the CryptoOutputStream closes the
+    // wrapped OutputStream even though we got an exception
+    // during CryptoOutputStream::close (in the flush method)
+    verify(outputStream).close();
+  }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-commits-h...@hadoop.apache.org