Java-Ressourcenverwaltung: Verständnis der Findbugs-Ergebnisse

Question

Findbugs bugs mich über eine Methode, die zwei Closeable Instanzen, aber ich kann nicht verstehen, warum.

Quelle

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-Analyse

Findbugs sagt mir

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

und zeigt auf die Linie mit FileReader fileReader = ...

Frage

Wer hat Unrecht: ich oder Findbugs?

Answer 1

FindBugs hat recht: Wenn der Konstruktor von FileWriter eine Ausnahme auslöst, wird der Dateileser nicht geschlossen. Um dies zu überprüfen, versuchen Sie, einen ungültigen Dateinamen für output .

Ich würde es folgendermaßen machen:

    FileReader fileReader = new FileReader(input);

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

Beachten Sie, dass die Behandlung von Ausnahmen, die beim Schließen ausgelöst werden, verbessert werden könnte, da das Verlassen eines finally-Blocks durch das Auslösen einer Ausnahme dazu führt, dass die try-Anweisung durch das Auslösen dieser Ausnahme beendet wird und jede Ausnahme, die im try-Block ausgelöst wird, verschluckt, was im Allgemeinen für die Fehlersuche nützlicher wäre. Siehe die Antwort von duffymo für eine einfache Möglichkeit, dies zu vermeiden.

bearbeiten : Seit Java 7 können wir die Anweisung try-with-resources verwenden, die eine korrekte und übersichtliche Behandlung dieser Eckfälle ermöglicht:

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

Answer 2

Das kann selbst für Findlinge kompliziert sein.

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

Ich habe den Eindruck, dass Sie recht haben.

EDITAR : Wow, ich dachte schon, ich werde abgewählt, weil ich gesagt habe, dass findbugs falsch liegen können!

EDITAR : Sieht so aus, als ob FindBugs doch recht hat. Guter Fang meriton.

Answer 3

Ich würde sagen, Sie sind es.

Ich würde beide Ressourcen in einem separaten Try/Catch-Block schließen. Ich würde statische Methoden erstellen, um mir zu helfen:

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

Ich denke, findbugs hat recht.

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

In diesem Block versuchen Sie, Ihren FileReader zu schließen. Dies kann jedoch eine Ausnahme auslösen, und in dem verschachtelten finally-Block schließen Sie den fileWriter. Haben Sie versucht, beide Reader im selben finally-Block zu schließen? Was sagt findbugs dann?

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