HADOOP-15085. Output streams closed with IOUtils suppressing write er…

…rors. Contributed by Jim Brennan
apache · Dec 14, 2017 · f8af0e2 · f8af0e2
1 parent 80db744
commit f8af0e2
Show file tree

Hide file tree

Showing 8 changed files with 38 additions and 69 deletions.
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileContext.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileContext.java
@@ -2121,17 +2121,13 @@ public boolean copy(final Path src, final Path dst, boolean deleteSource,
               content.getPath().getName())), deleteSource, overwrite);
         }
       } else {
-        InputStream in=null;
-        OutputStream out = null;
-        try {
-          in = open(qSrc);
-          EnumSet<CreateFlag> createFlag = overwrite ? EnumSet.of(
-              CreateFlag.CREATE, CreateFlag.OVERWRITE) : 
-                EnumSet.of(CreateFlag.CREATE);
-          out = create(qDst, createFlag);
+        EnumSet<CreateFlag> createFlag = overwrite ? EnumSet.of(
+            CreateFlag.CREATE, CreateFlag.OVERWRITE) :
+            EnumSet.of(CreateFlag.CREATE);
+        InputStream in = open(qSrc);
+        try (OutputStream out = create(qDst, createFlag)) {
           IOUtils.copyBytes(in, out, conf, true);
         } finally {
-          IOUtils.closeStream(out);
           IOUtils.closeStream(in);
         }
       }

diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java
@@ -1279,15 +1279,10 @@ public static String[] createJarWithClassPath(String inputClassPath, Path pwd,
 
     // Write the manifest to output JAR file
     File classPathJar = File.createTempFile("classpath-", ".jar", workingDir);
-    FileOutputStream fos = null;
-    BufferedOutputStream bos = null;
-    JarOutputStream jos = null;
-    try {
-      fos = new FileOutputStream(classPathJar);
-      bos = new BufferedOutputStream(fos);
-      jos = new JarOutputStream(bos, jarManifest);
-    } finally {
-      IOUtils.cleanupWithLogger(LOG, jos, bos, fos);
+    try (FileOutputStream fos = new FileOutputStream(classPathJar);
+         BufferedOutputStream bos = new BufferedOutputStream(fos)) {
+      JarOutputStream jos = new JarOutputStream(bos, jarManifest);
+      jos.close();
     }
     String[] jarCp = {classPathJar.getCanonicalPath(),
                         unexpandedWildcardClasspath.toString()};

diff --git a/...p-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CopyCommands.java b/...p-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CopyCommands.java
@@ -20,7 +20,6 @@
 
 import java.io.FileInputStream;
 import java.io.IOException;
-import java.io.InputStream;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.Iterator;
@@ -464,10 +463,8 @@ protected void processArguments(LinkedList<PathData> args)
         dst.fs.create(dst.path, false).close();
       }
 
-      InputStream is = null;
-      FSDataOutputStream fos = dst.fs.append(dst.path);
-
-      try {
+      FileInputStream is = null;
+      try (FSDataOutputStream fos = dst.fs.append(dst.path)) {
         if (readStdin) {
           if (args.size() == 0) {
             IOUtils.copyBytes(System.in, fos, DEFAULT_IO_LENGTH);
@@ -488,10 +485,6 @@ protected void processArguments(LinkedList<PathData> args)
         if (is != null) {
           IOUtils.closeStream(is);
         }
-
-        if (fos != null) {
-          IOUtils.closeStream(fos);
-        }
       }
     }
   }

diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/MapFile.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/MapFile.java
@@ -986,23 +986,22 @@ public static void main(String[] args) throws Exception {
     Configuration conf = new Configuration();
     FileSystem fs = FileSystem.getLocal(conf);
     MapFile.Reader reader = null;
-    MapFile.Writer writer = null;
     try {
       reader = new MapFile.Reader(fs, in, conf);
-      writer =
-        new MapFile.Writer(conf, fs, out,
-            reader.getKeyClass().asSubclass(WritableComparable.class),
-            reader.getValueClass());
-
       WritableComparable<?> key = ReflectionUtils.newInstance(
           reader.getKeyClass().asSubclass(WritableComparable.class), conf);
       Writable value = ReflectionUtils.newInstance(reader.getValueClass()
         .asSubclass(Writable.class), conf);
 
-      while (reader.next(key, value))               // copy all entries
-        writer.append(key, value);
+      try (MapFile.Writer writer = new MapFile.Writer(conf, fs, out,
+            reader.getKeyClass().asSubclass(WritableComparable.class),
+            reader.getValueClass())) {
+        while (reader.next(key, value)) {             // copy all entries
+          writer.append(key, value);
+        }
+      }
     } finally {
-      IOUtils.cleanupWithLogger(LOG, writer, reader);
+      IOUtils.cleanupWithLogger(LOG, reader);
     }
   }
 }
diff --git a/...op-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/nativeio/NativeIO.java b/...op-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/nativeio/NativeIO.java
@@ -956,28 +956,23 @@ public static void copyFileUnbuffered(File src, File dst) throws IOException {
     if (nativeLoaded && Shell.WINDOWS) {
       copyFileUnbuffered0(src.getAbsolutePath(), dst.getAbsolutePath());
     } else {
-      FileInputStream fis = null;
-      FileOutputStream fos = null;
+      FileInputStream fis = new FileInputStream(src);
       FileChannel input = null;
-      FileChannel output = null;
       try {
-        fis = new FileInputStream(src);
-        fos = new FileOutputStream(dst);
         input = fis.getChannel();
-        output = fos.getChannel();
-        long remaining = input.size();
-        long position = 0;
-        long transferred = 0;
-        while (remaining > 0) {
-          transferred = input.transferTo(position, remaining, output);
-          remaining -= transferred;
-          position += transferred;
+        try (FileOutputStream fos = new FileOutputStream(dst);
+             FileChannel output = fos.getChannel()) {
+          long remaining = input.size();
+          long position = 0;
+          long transferred = 0;
+          while (remaining > 0) {
+            transferred = input.transferTo(position, remaining, output);
+            remaining -= transferred;
+            position += transferred;
+          }
         }
       } finally {
-        IOUtils.cleanupWithLogger(LOG, output);
-        IOUtils.cleanupWithLogger(LOG, fos);
-        IOUtils.cleanupWithLogger(LOG, input);
-        IOUtils.cleanupWithLogger(LOG, fis);
+        IOUtils.cleanupWithLogger(LOG, input, fis);
       }
     }
   }

diff --git a/...-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java b/...-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
@@ -56,7 +56,6 @@
 import org.apache.hadoop.conf.Configuration.IntegerRanges;
 import org.apache.hadoop.fs.FileUtil;
 import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.io.IOUtils;
 import org.apache.hadoop.net.NetUtils;
 import org.apache.hadoop.security.alias.CredentialProvider;
 import org.apache.hadoop.security.alias.CredentialProviderFactory;
@@ -385,11 +384,9 @@ public void testMultiByteCharacters() throws IOException {
       Configuration conf = new Configuration(false);
       conf.addResource(new Path(CONFIG_MULTI_BYTE));
       assertEquals(value, conf.get(name));
-      FileOutputStream fos = new FileOutputStream(CONFIG_MULTI_BYTE_SAVED);
-      try {
+      try (FileOutputStream fos =
+               new FileOutputStream(CONFIG_MULTI_BYTE_SAVED)) {
         conf.writeXml(fos);
-      } finally {
-        IOUtils.closeStream(fos);
       }
 
       conf = new Configuration(false);

diff --git a/...p-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/compress/TestCodec.java b/...p-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/compress/TestCodec.java
@@ -362,11 +362,10 @@ private static Path writeSplitTestFile(FileSystem fs, Random rand,
     final Path file = new Path(wd, "test" + codec.getDefaultExtension());
     final byte[] b = new byte[REC_SIZE];
     final Base64 b64 = new Base64(0, null);
-    DataOutputStream fout = null;
     Compressor cmp = CodecPool.getCompressor(codec);
-    try {
-      fout = new DataOutputStream(codec.createOutputStream(
-            fs.create(file, true), cmp));
+    try (DataOutputStream fout =
+             new DataOutputStream(codec.createOutputStream(fs.create(file,
+                 true), cmp))) {
       final DataOutputBuffer dob = new DataOutputBuffer(REC_SIZE * 4 / 3 + 4);
       int seq = 0;
       while (infLen > 0) {
@@ -382,7 +381,6 @@ private static Path writeSplitTestFile(FileSystem fs, Random rand,
       }
       LOG.info("Wrote " + seq + " records to " + file);
     } finally {
-      IOUtils.cleanupWithLogger(LOG, fout);
       CodecPool.returnCompressor(cmp);
     }
     return file;

diff --git a/...mon-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/MiniKMS.java b/...mon-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/MiniKMS.java
@@ -105,15 +105,11 @@ public MiniKMS(String kmsConfDir, String log4ConfFile, String keyStore,
 
   private void copyResource(String inputResourceName, File outputFile) throws
       IOException {
-    InputStream is = null;
-    OutputStream os = null;
-    try {
-      is = ThreadUtil.getResourceAsStream(inputResourceName);
-      os = new FileOutputStream(outputFile);
+    InputStream is = ThreadUtil.getResourceAsStream(inputResourceName);
+    try (OutputStream os = new FileOutputStream(outputFile)) {
       IOUtils.copy(is, os);
     } finally {
       IOUtils.closeQuietly(is);
-      IOUtils.closeQuietly(os);
     }
   }