]> source.dussan.org Git - jgit.git/commit
Git: Don't close underlying repo if it came from from a caller 50/44050/2
authorDave Borowitz <dborowitz@google.com>
Tue, 17 Mar 2015 22:24:59 +0000 (15:24 -0700)
committerDave Borowitz <dborowitz@google.com>
Wed, 18 Mar 2015 00:05:55 +0000 (17:05 -0700)
commitfd402f71a3c7cfd62d3ec0963e803ecbb875d210
treefb972e8d311443a417f47254786536af8cb2a039
parent3ac06ca6de3fc175ddac5e16d5313bf84b70936f
Git: Don't close underlying repo if it came from from a caller

Since 27ae8bc65 Git has implemented AutoCloseable, which means Eclipse
may warn if close() is never called on a Git instance. For example,
the following would result in a resource warning:

  Repository repo = openRepository(foo);
  Git git = new Git(repo);
  try {
    git.someCommand().call();
  } finally {
    repo.close();
  }

(The same warning would occur if repo were created in a try-with-
resources block.)

The "obvious" fix is to open git in a try-with-resources block:

  try (Repository repo = openRepository(foo);
      Git git = new Git(repo)) {
    git.someCommand().call();
  }

Unfortunately, this construction was subtly broken: it would call both
git.close() and repo.close(), but git.close() would call repo.close()
again. Depending on the repository implementation, this might or might
not be ok. If it's not ok, it might not immediately cause an error, if
the reference count of repo was >2 at the time of closing.

Of course, explicitly calling git.close() followed by repo.close() in
two finally blocks has had the same double-closing problem since
forever. But the problem became worse when Git started implementing
AutoCloseable, because now Eclipse is _actively encouraging_
developers to change working code into broken code.

To work around this, keep track in Git's constructor of whether the
repository was passed in or opened at construction time, and only
close the repository if it was opened by Git.

Note that in the original example, there was not _actually_ a resource
leak, since repo was closed exactly once; git did not _need_ to be
closed in this case. But at least fixing this false-positive warning
no longer introduces a real bug.

Change-Id: Ie927a26ce3ae2bf8c3ef5cb963a60847067db95a
org.eclipse.jgit/src/org/eclipse/jgit/api/Git.java