Will not closing a stringwriter cause a leak?

Question

I realize that in java the GC will eventually cleanup objects, but I'm asking if it is bad practice to not close your string writer, currently I am doing this:

 private static String processTemplate(final Template template, final Map root) {
        StringWriter writer = new StringWriter();
        try {
            template.process(root, writer);
        } catch (TemplateException e) {
            logger.error(e.getMessage());
        } catch (IOException e) {
            logger.error(e.getMessage());
        }
        finally {

        }

        return writer.toString();
    }

Should I be closing the writer and creating a new String like this:

String result = "";

...

finally {
  result = writer.toString();
  writer.close();
}

Is this better to do?

Answer 1

The javadoc is quite explicit:

Closing a StringWriter has no effect.

And a quick look at the code confirms it:

public void close() throws IOException {
}

Answer 2

It's not holding any non-memory resource. It will be garbage collected like anything else. The close() probabaly merely exists because other writer objects do hold resources that need to be cleaned up, and the close() is needed to satify the interface.

Answer 3

No, not closing a StringWriter will not cause a leak: as noted, StringWriter#close() is a nop, and the writer only holds memory, not external resources, so these will be collected when the writer is collected. (Explicitly, it holds references to objects in private fields that do not escape the object, concretely a StringBuffer, so no outside references.)

Further, you generally shouldn't close a StringWriter, because it adds boilerplate to your code, obscuring the main logic, as we'll see. However, to reassure readers that you're being careful and doing this intentionally, I'd recommend commenting this fact:

// Don't need to close StringWriter, since no external resource.
Writer writer = new StringWriter();
// Do something with writer.

If you do want to close the writer, most elegant is to use try-with-resources, which will automatically call close() when you exit the body of the try block:

try (Writer writer = new StringWriter()) {
    // Do something with writer.
    return writer.toString();
}

However, since Writer#close() throws IOException, your method now needs to also throw IOException even though it never occurs, or you need to catch it, to prove to the compiler that it is handled. This is quite involved:

Writer writer = new StringWriter();
try {
    // Do something with writer, which may or may not throw IOException.
    return writer.toString();
} finally {
    try {
        writer.close();
    } catch (IOException e) {
        throw new AssertionError("StringWriter#close() should not throw IOException", e);
    }
}

This level of boilerplate is necessary because you can't just put a catch on the overall try block, as otherwise you might accidentally swallow an IOException thrown by the body of your code. Even if there isn't any currently, some might be added in future and you'd want to be warned of this by the compiler. The AssertionError is documenting the current behavior of StringWriter#close(), which could potentially change in a future release, though that is extremely unlikely; it also masks any exception that may occur in the body of the try (again, this should never occur in practice). This is far too much boilerplate and complexity, and you'd clearly be better off omitting the close() and commenting why.

A subtle point is that not only does Writer#close() throw an IOException, but so does StringWriter#close(), so you can't eliminate the exception by making the variable a StringWriter instead of a Writer. This is different from StringReader, which overrides the close() method and specifies that it does not throw an exception! See my answer to Should I close a StringReader?. This may look wrong – why would you have a method that does nothing but may throw an exception?? – but is presumably for forward compatibility, to leave open the possibility of throwing an IOException on close in future, as this is an issue for writers generally. (It could also just be a mistake.)

To summarize: it's fine to not close a StringWriter, but the reason to not do the usual right thing, namely try-with-resources, is just because close() declares that it throws an exception that it doesn't actually throw in practice, and handling this precisely is a lot of boilerplate. In any other case it's better to just use the conventionally correct resource-management pattern and prevent problems and head-scratching.

Answer 4

At the end of the method there is no reference left to the writer therefore it will be freed by the GC.