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
This commit is contained in:
gardenia 2023-02-07 12:01:57 +00:00 committed by GitHub
parent f02c452cf1
commit 8714403dc7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 28 additions and 5 deletions

View File

@ -240,13 +240,16 @@ public synchronized void close() throws IOException {
if (closed) { if (closed) {
return; return;
} }
try {
try { try {
flush(); flush();
} finally {
if (closeOutputStream) { if (closeOutputStream) {
super.close(); super.close();
codec.close(); codec.close();
} }
freeBuffers(); freeBuffers();
}
} finally { } finally {
closed = true; closed = true;
} }

View File

@ -17,12 +17,14 @@
*/ */
package org.apache.hadoop.crypto; package org.apache.hadoop.crypto;
import java.io.IOException;
import java.io.OutputStream; import java.io.OutputStream;
import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.conf.Configuration;
import org.junit.BeforeClass; import org.junit.BeforeClass;
import org.junit.Test; import org.junit.Test;
import static org.apache.hadoop.test.LambdaTestUtils.intercept;
import static org.mockito.Mockito.*; import static org.mockito.Mockito.*;
/** /**
@ -54,4 +56,22 @@ public void testOutputStreamNotClosing() throws Exception {
verify(outputStream, never()).close(); 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();
}
} }