Mehrere resources mit try-with-resources verwalten

[Marco13]

In einem anderen Thread hatte ich erwähnt, dass ich über dieses Konstrukt hier gestolpert war:

private void writeResourceFile(String binaryName, FileObject itemResource) throws IOException {
    try (OutputStream os = itemResource.openOutputStream()) {
        try (BufferedOutputStream bos = new BufferedOutputStream(os)) {
            try (OutputStreamWriter osw = new OutputStreamWriter(bos, StandardCharsets.UTF_8)) {
                try (BufferedWriter bw = new BufferedWriter(osw)) {
                    bw.write(binaryName);
                    bw.newLine();
                }
            }
        }
    }
}

Und es hat sich die Frage ergeben, wie man das denn nun „richtig“ oder „besser“ macht. Ich hatte erwähnt, dass ich nicht sicher bin, ob das obige schlimmer wäre, oder das hier:

    try (BufferedWriter bw = new BufferedWriter(
             new OutputStreamWriter(
                 new BufferedOutputStream(
                     itemResource.openOutputStream()), 
                 StandardCharsets.UTF_8))) {
        bw.write(binaryName);
        bw.newLine();
    }

Die weiteren Beiträge, die sich darauf bezogen, schieb ich hier mal ein:

[Landei]

Ist ein kleines bisschen OT, aber ich würde es so schreiben:

try (BufferedOutputStream bos = new BufferedOutputStream(itemResource.openOutputStream());
     OutputStreamWriter osw = new OutputStreamWriter(bos);
     BufferedWriter bw = new BufferedWriter(osw, StandardCharsets.UTF_8)) {
         bw.write(binaryName);
         bw.newLine();
}

[TMII]

In deinem Beispiel schließt try(BufferedOutputStream) zwar den übergebenen OutputStream gleich mit. Aber OutputStreamWriter und BufferedWriter werden nicht vorher geschlossen. Kann also sein, dass sich noch ungeschriebene Daten in der Pipeline befinden.

Würdest du BufferedWriter in den try-Block legen, schließt der wiederum nicht alle darunter liegenden Streams und wir wissen alle wohin ungeschlossene Streams führen können.

Von daher ist die Google-Methode schon richtig. Er schließt die Streams, nacheinander, von oben nach unten.

@Marco13

try (BufferedWriter bw = new BufferedWriter(
             new OutputStreamWriter(
                 new BufferedOutputStream(
                     itemResource.openOutputStream()), 
                 StandardCharsets.UTF_8))) {
    }

Sicher dass das so funktioniert? Meiner Meinung müsste man das wenn dann so schreiben:

try (BufferedOutputStream bos = new BufferedOutputStream(itemResource.openOutputStream());
OutputStreamWriter osw = new OutputStreamWriter(bos, StandardCharsets.UTF_8);
BufferedWriter bw = new BufferedWriter(osw)) {
}

wobei ich nicht sicher bin, ob die auch in der richtigen Reihenfolge geschlossen werden?

[CyborgBeta]

@TMII

In deinem Beispiel schließt try(BufferedOutputStream) zwar den übergebenen OutputStream gleich mit. Aber OutputStreamWriter und BufferedWriter werden nicht vorher geschlossen

Bei Landeis Methode werden alle Streams/Ressourcen in umgekehrt Erstellungsreihenfolge geschlossen. Also das, was erwünscht ist…

/e Auch bei Fehlerfällen.

[TMII]

@CyborgBeta

Bei Landeis Methode werden alle Streams/Ressourcen in umgekehrt Erstellungsreihenfolge geschlossen. Also das, was erwünscht ist…

Das sehe ich nicht in der Javadoc. Weder AutoClosable, noch Flushable, noch BufferedOutputStream, OutputStreamWriter oder BufferedWriter beschreiben das Verhalten das du beschreibst.

Nebenbei zu der Frage die ich mir selbst gestellt hatte,

@TMII

wobei ich nicht sicher bin, ob die auch in der richtigen Reihenfolge geschlossen werden?

Oracle JavaDoc

Note that the close methods of resources are called in the opposite order of their creation.

[Marco13]

Unter The try-with-resources Statement (The Java™ Tutorials > Essential Java Classes > Exceptions) gibt’s da ein Beispiel:

try (
    java.util.zip.ZipFile zf =
         new java.util.zip.ZipFile(zipFileName);
    java.io.BufferedWriter writer = 
        java.nio.file.Files.newBufferedWriter(outputFilePath, charset)
) { ...  }

mit dem Kommentar

In this example, the try -with-resources statement contains two declarations that are separated by a semicolon: ZipFile and BufferedWriter . When the block of code that directly follows it terminates, either normally or because of an exception, the close methods of the BufferedWriter and ZipFile objects are automatically called in this order. Note that the close methods of resources are called in the opposite order of their creation.

Wobei man das hier noch genauer auseinander pflücken müßte: In den Beispiel sind die resourcen komplett unabhängig. Aber bei diesen verschachtelten Konstrukten, d.h. sowas wie new BufferedReader(someReader) ist es IIRC oft so, dass das „innere“ (also der someReader) auch geschlossen wird, wenn das „äußere“ geschlossen wird. (Aber zu Ausnahmen müßte ich nochmal nachlesen…)

EDIT: Eigentlich wollte ich noch erwähnen, dass close() von Closeable ja idempotent ist, aber ich habe gerade gesehen, dass das bei AutoCloseable nicht zugesichert (sondern nur „dringend empfohlen“) ist - hm. Aber soweit ich weiß sind die Dinge, um die es hier bisher ging, auch Closeable.

[CyborgBeta]

Also,
A es wäre seltsam, wenn sie Multi-try-with-resources eingeführt hätten, dabei die Streams aber nicht geschlossen hätten/würden,
B ein doppelter close-Aufruf ist doch gar nicht weiter schlimm,
C imho ist

try (FileReader fr = new FileReader(""); FileWriter fw = new FileWriter("")) {
	int c = fr.read();
	if (c != -1) {
		fw.write(c);
	}
} catch (IOException ignored) {
}

nur eine „Umschreibung“ für

{
	FileReader fr = null;
	FileWriter fw = null;
	try {
		fr = new FileReader("");
		fw = new FileWriter("");
		int c = fr.read();
		if (c != -1) {
			fw.write(c);
		}
	} catch (IOException ignored) {
	} finally {
		if (fw != null) {
			try {
				fw.flush();
			} catch (IOException ignored) {
			} finally {
				try {
					fw.close();
				} catch (IOException ignored) {
				}
			}
		}
		if (fr != null) {
			try {
				fr.close();
			} catch (IOException ignored) {
			}
		}
	}
}

was dann doch etwas viel Boilerplate wäre…

[TMII]

Landei hat den zweit-untersten Stream geschlossen. BufferedOutputStream. Schauen wir in den Sourcecode was bei close() passiert, steht dort:

public void close() throws IOException {
        try {
          flush();
        } catch (IOException ignored) {
        }
        out.close();
    }

er flusht und schließt den darunter liegenden Stream. Das Verhalten steht auch so im JavaDoc für BufferedOutputStream.close() - und nur dort.

Er schließt aber nicht nach oben. Und das führt dann zu dem folgenden Problem, dass die oberen Streams beim schließen nicht geflusht werden.

try( RandomStream stream = new RandomStream() ) {
    AnotherStream another = new AnotherStream();
    another.write( data );
}

nach dem try-Block wird RandomStream geschlossen, aber „data“ kann sich immer noch in AnotherStream befinden.

Aber zu dem anderen Thema (von Marco), auch die Methode von oben nach unten ist brandgefährlich, denn die JavaDocs erwähnen das Verhalten mit keinem Wort. Und obwohl die meisten IO-Streams der Oracle Standardbibliothek das Verhalten still und heimlich implementieren wenn man in den Source-Code schaut, ist es weder vorgeschrieben noch erwähnt.

Manch eine Klasse wie der XMLOutputStream schreibt explizit:

Close this writer and free any resources associated with the writer. This must not close the underlying output stream.

Auch die Java 8 Streams schließen meist nicht ihren Sourcestream, es sei denn es wurde explizit festgelegt, wie beispielsweise bei Files.list().

Ich will damit nur sagen, man sollte sich nicht auf etwas verlassen, was nicht zugesichert wurde, nur weil es halt schon immer so war.

[Landei]

@TMII

Landei hat den zweit-untersten Stream geschlossen.

??? parse error ???

In meinem Codeschnipsel werden alle drei Closables aus dem try-Header (BufferOutputStream, OutStreamWriter, BufferedWriter) am Ende geschlossen.

[TMII]

@Landei

??? parse error ???

In meinem Codeschnipsel werden alle drei Closables aus dem try-Header (BufferOutputStream, OutStreamWriter, BufferedWriter) am Ende geschlossen.

ahja stimmt. Keine Ahnung wie ich das übersehen konnte.

[CyborgBeta]

@TMII

er flusht und schließt den darunter liegenden Stream

@TMII

Er schließt aber nicht nach oben

@TMII

brandgefährlich

Na ja, das ist richtig! Und wenn man die Streams „richtigherum zusammensetzt“, dann sehe ich darin gar kein Problem. … Du solltest dich nicht so aufregen… Oder du solltest eine Fliege tragen (die wirbelt auch viel Luft herum). (Soll keine Provokation sein).

[Marco13]

Hm. Was genau nun „brandgefährlich“ ist, hat sich mir nicht erschlossen. Aber ich finde (auch, falls du das meintest) dass einiges im Dunstkreis von streams unerfreulich vage ist:

• Bei den meisten stream-Wrappern wird bei einem close auch der „innere“ Stream geschlossen (aber wohl nicht bei allen)
• Bei den meisten streams ist close idempotent (aber wohl nicht bei allen)
• Bei den meisten streams wird bei einem close erst ein flush gemacht (aber vielleicht nicht bei allen…?)
• Nebnbei: Auch ein normaler java.util.Stream hat ein close, und bei manchen ist es nötig, das aufzurufen, aber wohl nicht bei allen…

^{(Da bin ich vor einer Weile drüber gestolpert, als ich für data die Möglichkeit bieten wollte, einzelne Zeilen einer CSV über einen Stream zu verarbeiten. Das blöde ist: Wenn man einen Stream in die Hand gedrückt bekommt, weiß man nie, ob man ihn schließen muss. Man könnte meinen ~„joa, dann macht man halt was, dass er bei einer terminal operation geschlossen wird“, aber so einfach ist das alles nicht…)}

Tatsächlich kann aber z.B. auch das explizite Schließen eines „eingewickelten“ Streams richtig eklig sein. Ganz grob (wirklich nur sinngemäß) bei so einem Muster:

void writeSomething(OutputStream outputStream) {
    writeSomeXml(outputStream);
    writeSomeJson(outputStream);
}
void writeSomeXml(OutputStream outputStream) {
    try (XmlWriter w = new XmlWriter(outputStream)) { ... }
}
void writeSomeJson(OutputStream outputStream) {
    try (JsonWriter w = new JsonWriter(outputStream)) { ... }
}

Wenn man mehrere Sachen hintereinander in einen Stream schreiben will, kann ein automatisches Schließen lästig sein. IIRC habe ich sogar mal was mit

class NonCloseableOutputStream implements OutputStream {
    OutputStream delegate;
    void write(byte data[]) { delegate.write(data);
    void close() { /** delegate.close()? NO! */ }
}

gepfuscht, weil’s keine andere Lösung zu geben schien (gab es IIRC aber dann doch…).

EDIT: Ganz allgemein versuche ich aber immer, wenn ein InputStream oder OutputStream an eine Methode übergeben wird, klipp und klar dazuzuschreiben, ob…

/**
 * ...
 * The given stream will be closed when the operation is finished
 */

oder

/**
 * ...
 * The caller is responsible for closing the given stream
 */

Und nach Möglichkeit mache ich letzteres: Ein stream, der einmal zu ist, den kriegt man halt nicht mehr auf. (Lästig ist es dann, wenn irgendeine Library das erstens nicht macht, und zweitens die Bedingungen, unter denen etwas geschlossen wird, haarsträubend undurchsichtig sind … )

[CyborgBeta]

Also seit vielen Jahren /seit es try with resources gibt/seit Java 7(?) hab ich überhaupt keine Probleme mehr bezüglich flush oder close… Man muss schon sehr „ungeschickt“ sein, um dabei etwas falsch zu machen. - Das kann aber auch daran liegen, dass ich seltenst Streams an Methoden delegiere… Also nur sehr lokal verwende.

[nolle]

Ich bin ein Fan von Project Lombok. Mit @Cleanup spart man sich die try-Blöcke.
https://projectlombok.org/features/Cleanup

[Clayn]

Das beispiel ist aber auch extra hässlich gemacht, das würde man ja so heutzutage nicht mehr machen