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

Zhe Zhang commented on HADOOP-12615:
------------------------------------

Thanks Steve for the suggestion; good point.

Thank you for the update Wei-Chiu. I have a few questions/comments.
# I'm not sure if {{cl = ThreadUtil.class.getClassLoader()}} is the right 
behavior. Could you explain why we can use the CL of {{ThreadUtil}} to replace 
the CL of the current thread?
# In {{MiniKMS}}, better make the following 2 segments consistent. They should 
both use a try-finally structure, or neither. Actually I don't think we need 
the try-finally: in case of IOException, {{is}} remains null anyway.
{code}
    if (!aclsFile.exists()) {
      InputStream is =
          ThreadUtil.getResourceAsStream("mini-kms-acls-default.xml");
{code}
{code}
      InputStream is = null;
      OutputStream os;
      try {
        is = ThreadUtil.getResourceAsStream("kms-webapp/WEB-INF/web.xml");
        os = new FileOutputStream(new File(webInf, "web.xml"));
        IOUtils.copy(is, os);
      } finally {
        IOUtils.closeQuietly(is);
        IOUtils.closeQuietly(is);
      }
{code}
# Also, {{is}} is closed twice in the above. A typo with {{os}}?
# The local variable {{is}} in {{TestKMSAudit.java}} doesn't look necessary? 
How about keeping the original structure but use the new 
{{getResourceAsStream}} method?
# {{MiniKdc}} still uses the old {{getResourceAsStream}}

> Fix NPE in MiniKMS.start()
> --------------------------
>
>                 Key: HADOOP-12615
>                 URL: https://issues.apache.org/jira/browse/HADOOP-12615
>             Project: Hadoop Common
>          Issue Type: Bug
>          Components: kms, test
>    Affects Versions: 3.0.0
>            Reporter: Wei-Chiu Chuang
>            Assignee: Wei-Chiu Chuang
>            Priority: Minor
>              Labels: jenkins, supportability, test
>         Attachments: HADOOP-12615.001.patch, HADOOP-12615.002.patch, 
> HADOOP-12615.003.patch, HADOOP-12615.004.patch, HADOOP-12615.005.patch
>
>
> Sometimes, KMS resource file can not be loaded. When this happens, an 
> InputStream variable will be a null pointer which will subsequently throw NPE.
> This is a supportability JIRA that makes the error message more explicit, and 
> explain why NPE is thrown. Ultimately, leads us to understand why the 
> resource files can not be loaded.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to