0

try-with-resources is nice and all that, but it seems to me that it is still not sufficient for effective resource management when creating classes that wrap multiple AutoCloseable objects. For example, consider

import java.io.*;

class AutocloseableWrapper implements AutoCloseable {
    private FileReader r1;
    private FileReader r2;

    public AutocloseableWrapper(String path1, String path2) throws IOException {
        r1 = new FileReader(path1);
        r2 = new FileReader(path2);
    }

    @Override
    public void close() throws IOException {
        r1.close();
        r2.close();
    }

    public static void main(String[] args) throws IOException {
        try (AutocloseableWrapper w = new AutocloseableWrapper("good-path", "bad-path")) {
                System.out.format("doing something\n");
                throw new IOException("doing something in main");
            }
    }
}

There are at least two issues with this wrapper:

  1. If "bad-path" is invalid and causes the assignment to r2 to throw, then r1 is not closed.
  2. If wrapper construction succeeds but then r1.close throws, then r2 is not closed.

All those issues can be addressed, but then writing the wrapper becomes quite non-trivial and error-prone, even if wrapping only two resources:

import java.io.*;

class AutocloseableWrapper implements AutoCloseable {
    private FileReader r1;
    private FileReader r2;

    public AutocloseableWrapper(String path1, String path2) throws IOException {
        r1 = new FileReader(path1);
        try {
            r2 = new FileReader(path2);
        }
        catch (IOException e) {
            try {
                r1.close();
            }
            catch (IOException e2) {
                e.addSuppressed(e2);
            }
            throw e;
        }
    }

    @Override
    public void close() throws IOException {
        IOException e = null;
        try {
            r1.close();
        }
        catch (IOException e1) {
            e = e1;
        }

        try {
            r2.close();
        }
        catch (IOException e2) {
            if (e == null)
                throw e2;
            else {
                e.addSuppressed(e2);
                throw e;
            }
        }
    }

    public static void main(String[] args) throws IOException {
        try (AutocloseableWrapper w = new AutocloseableWrapper("good-path", "bad-path")) {
                System.out.format("doing something\n");
                throw new IOException("doing something in main");
            }
    }
}

Is there some helper class or any other way to make writing wrappers easier?

Deduplicator
  • 44,692
  • 7
  • 66
  • 118
Paulo
  • 757
  • 8
  • 18
  • 1
    Put the outer close in a finally so that thrown RuntimeExceptions don't also cause you to leak resources. – David Ehrmann Jul 11 '14 at 19:27
  • @DavidEhrmann, thank you for pointing out! This is one more reason why I'm frustrated at having to write wrappers like that. – Paulo Jul 11 '14 at 20:06
  • @Paulo You should catch with the "catch-all" Throwable catch clause and then rethrow it.This would still work because of precise rethrow feature in Java7 exclusively introduced because of try-with-resources statement – Kumar Abhinav Jul 11 '14 at 20:31

2 Answers2

2

You should enable the syntactic code unwrapped by the compiler....You can find the Oracle article over here :- http://www.oracle.com/technetwork/articles/java/trywithresources-401775.html

Coming to the question,if you have a wrapper you can do something like this

@Override
public void close() throws IOException {
    Throwable t = null;
    try {
        r1.close();
    } catch (Throwable t1) {
        t = t1;
        throw t1;
    } finally {
        if (t != null) {
            try {
                r2.close();
            } catch (Throwable t2) {
                t.addSuppressed(t2);
            }
        } else {

            r2.close();
        }
    }
}

Note:This will work because of precise rethrow feature in Java 7

Kumar Abhinav
  • 6,565
  • 2
  • 24
  • 35
  • Thank you for the ref to the article. I can see that your code is much more correct than mine and that it follows the patterns that the compiler would generate for a try-with-resources according to the article. However, if I understood correctly, this is something we still will have to code by hand inside the close and constructor methods of the wrapper. Is that so? – Paulo Jul 11 '14 at 22:09
  • @Paulo..In case of a wrapper,you already have the resources instantiated and therefore you would need to implement your own custom close method..However,If you need to instantiate resources and then close them,you can use try-with-resources and you would not have to implement your own close functionality – Kumar Abhinav Jul 12 '14 at 04:06
1

You could use a generic resource wrapper such as:

public class CloseableChain implements AutoCloseable {
  private AutoCloseable r1;
  private CloseableChain r2;

  public void attach(AutoCloseable r) {
    if (r1 == null) {
      r1 = r;
    } else {
      if (r2 == null) {
        r2 = new CloseableChain();
      }
      r2.attach(r);
    }
  }

  public void close() throws Exception {
    if (r1 == null) {
      return;
    }
    Throwable t = null;
    try {
      r1.close();
    } catch (Throwable t1) {
      t = t1;
      throw t1;
    } finally {
      if (r2 != null) {
        if (t != null) {
          try {
            r2.close();
          } catch (Throwable t2) {
            t.addSuppressed(t2);
          }
        } else {
          r2.close();
        }
}}}}

Then you could refactor your code to:

import java.io.*;

class AutocloseableWrapper implements AutoCloseable {
  private CloseableChain chain;
  private FileReader r1;
  private FileReader r2;
  private FileReader r3;

  public AutocloseableWrapper(String path1, String path2) throws IOException {
    chain = new CloseableChain();
    r1 = new FileReader(path1);
    chain.attach(r1);
    r2 = new FileReader(path2);
    chain.attach(r2);
    // and even more...
    r3 = new FileReader("whatever");
    chain.attach(r3);
  }

  @Override
  public void close() throws IOException {
    chain.close();
  }

  public static void main(String[] args) throws IOException {
    try (AutocloseableWrapper w = new AutocloseableWrapper("good", "bad")) {
      System.out.format("doing something\n");
      throw new IOException("doing something in main");
    }
  }
}
Marc-Christian Schulze
  • 3,154
  • 3
  • 35
  • 45
  • Very nice indeed, @coding.mof! It happened to me that I in fact started exploring this approach more thoroughly ever since I started pondering on the issue. Please, allow me here to shamelessly plug [my own take at that](https://github.com/prosoft-nearshore/Java-Scopes). – Paulo Dec 01 '15 at 19:25