Управление ресурсами Java: понимание результатов Findbugs

Question

Findbugs сообщает мне о методе, который открывает два Closeable экземпляра, но я не могу понять, почему.

Источник

public static void sourceXmlToBeautifiedXml(File input, File output)
        throws TransformerException, IOException, JAXBException {

    FileReader fileReader = new FileReader(input);
    FileWriter fileWriter = new FileWriter(output);

    try {
        // may throw something
        sourceXmlToBeautifiedXml(fileReader, fileWriter);
    } finally {
        try {
            fileReader.close();
        } finally {
            fileWriter.close();
        }
    }
}

Анализ Findbugs

Findbugs говорит мне

Method [...] may fail to clean up java.io.Reader [...]

и указывает на строку с FileReader fileReader = ...

Вопрос

Кто не прав: я или Findbugs?

Answer 1

FindBugs правильно: если конструктор FileWriter выдает исключение, программа чтения файлов не будет закрыта. Чтобы убедиться в этом, попробуйте передать неверное имя файла для output.

Я бы сделал это следующим образом:

    FileReader fileReader = new FileReader(input);

    try {
        FileWriter fileWriter = new FileWriter(output);
        try {
            // may throw something
            sourceXmlToBeautifiedXml(fileReader, fileWriter);
        } finally {
            fileWriter.close();
        }
    } finally {
        fileReader.close();
    }

Обратите внимание, что обработка исключения, создаваемого при закрытии, могла бы быть улучшена, поскольку выход из блока finally с использованием исключения приведет к завершению оператора try путем выброса этого исключения, проглотив любое исключение, выброшенное в блоке try, как правило, было бы более полезно для отладки. Смотрите ответ Даффимо, чтобы узнать, как этого избежать.

Редактировать : Начиная с Java 7, мы можем использовать инструкцию try-with-resources, которая разрешает правильную и краткую обработку следующих угловых случаев:

try (
    FileReader fileReader = new FileReader(input); 
    FileWriter fileWriter = new FileWriter(output)
) {
    // may throw something
    sourceXmlToBeautifiedXml(fileReader, fileWriter);
}

Answer 2

Это может быть сложно даже для findbugs.

try {
   fileReader.close();
} finally {
   fileWriter.close();
}

Мне кажется, ты прав.

РЕДАКТИРОВАТЬ : Ух ты, я думал, что меня проголосуют за то, что findbugs могут ошибаться!

РЕДАКТИРОВАТЬ : Похоже, что FindBugs прав в конце концов. Хороший улов меритон.

Answer 3

я бы сказал, что это ты.

Я бы закрыл оба ресурса в отдельном блоке try / catch. Я бы создал статические методы, чтобы помочь мне:

public static void sourceXmlToBeautifiedXml(File input, File output)
        throws TransformerException, IOException, JAXBException {

    FileReader fileReader = new FileReader(input);
    FileWriter fileWriter = new FileWriter(output);

    try {
        // may throw something
        sourceXmlToBeautifiedXml(fileReader, fileWriter);
    } finally {
        close(fileReader);
        close(fileWriter);
    }
}


// same for reader & writer
public static void close(InputStream s)
{
    try
    { 
       if (s != null)
       {
           s.close();
       }
    }
    catch (IOException e)
    {
        e.printStackTrace();
    }
}

Answer 4

Я думаю, что findbugs прав.

} finally {
    try {
        fileReader.close();
    } finally {
        fileWriter.close();
    }
}

В этом блоке вы пытаетесь закрыть свой FileReader. Это, однако, может вызвать исключение, и в конце вложенного файла вы закроете fileWriter. Вы пытались закрыть оба читателя в одном и том же блоке finally? Что же тогда говорят findbugs?

} finally {
    try {
        fileReader.close();
        fileWriter.close();
    } finally {
        //dunno maybe log that something went wrong.
    }
}