[ 
https://issues.apache.org/jira/browse/HADOOP-15085?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16288393#comment-16288393
 ] 

Jason Lowe commented on HADOOP-15085:
-------------------------------------

Thanks for updating the patch!

In the FileContext change there's a subtle semantic difference after the 
change.  Previously it opened the input stream and then created the output 
file, but now it does the reverse.  If we are unable to open the input file 
then an empty output file will be left around.  The open call should be moved 
before the try-with-resource block.

The following code:
{code}
      JarOutputStream jos = null;
      jos = new JarOutputStream(bos, jarManifest);
      if (jos != null) {
        jos.close();
      }
{code}

should be simplified to the following since constructors cannot fail without 
throwing:
{code}
      JarOutputStream jos = new JarOutputStream(bos, jarManifest);
      jos.close();
{code}

The NativeIO change and MiniKMS changes have a similar subtle ordering issue as 
the FileContext change.  The construction of the input stream should occur 
before the try-with-resource block for the output stream.

> Output streams closed with IOUtils suppressing write errors
> -----------------------------------------------------------
>
>                 Key: HADOOP-15085
>                 URL: https://issues.apache.org/jira/browse/HADOOP-15085
>             Project: Hadoop Common
>          Issue Type: Bug
>            Reporter: Jason Lowe
>            Assignee: Jim Brennan
>         Attachments: HADOOP-15085.001.patch, HADOOP-15085.002.patch, 
> HADOOP-15085.003.patch, HADOOP-15085.004.patch
>
>
> There are a few places in hadoop-common that are closing an output stream 
> with IOUtils.cleanupWithLogger like this:
> {code}
>   try {
>     ...write to outStream...
>   } finally {
>     IOUtils.cleanupWithLogger(LOG, outStream);
>   }
> {code}
> This suppresses any IOException that occurs during the close() method which 
> could lead to partial/corrupted output without throwing a corresponding 
> exception.  The code should either use try-with-resources or explicitly close 
> the stream within the try block so the exception thrown during close() is 
> properly propagated as exceptions during write operations are.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to