commit 846bd6a5278b2b949bdd7f77bca01fedf29778b3
author David Pursehouse <david.pursehouse@sonymobile.com> Thu May 21 11:31:32 2015 +0900
committer David Pursehouse <david.pursehouse@sonymobile.com> Fri May 22 00:44:26 2015 +0000
tree a5273e63ba2f7f7a7102a5861d553ce84282908d
parent b5c6f81d979e78fbd734d946b18270ec8319eaf6

Use auto-closeable RevWalk when creating review notes

In CreateReviewNotes.createNotes a RevWalk is created, but does not get
closed if an exception occurs when parsing the commit or marking objects
as uninteresting because the method just returns in the catch block. The
RevWalk is only closed in the later finally block that is not reached in
this case.

Enclose both blocks in a try-with-resources for the RevWalk, to ensure
that it always gets closed.

Change-Id: Ief04c620ffe664dd9df5527473fc1a5e6354aa33

src/main/java/com/googlesource/gerrit/plugins/reviewnotes/CreateReviewNotes.java

diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/CreateReviewNotes.java b/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/CreateReviewNotes.java
index 853585d..72c9ec9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/CreateReviewNotes.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/CreateReviewNotes.java
@@ -129,25 +129,24 @@
       return;
     }
 
-    RevWalk rw = new RevWalk(git);
-    try {
-      RevCommit n = rw.parseCommit(newObjectId);
-      rw.markStart(n);
-      if (n.getParentCount() == 1 && n.getParent(0).equals(oldObjectId)) {
-        rw.markUninteresting(rw.parseCommit(oldObjectId));
-      } else {
-        markUninteresting(git, branch, rw, oldObjectId);
+    try (RevWalk rw = new RevWalk(git)) {
+      try {
+        RevCommit n = rw.parseCommit(newObjectId);
+        rw.markStart(n);
+        if (n.getParentCount() == 1 && n.getParent(0).equals(oldObjectId)) {
+          rw.markUninteresting(rw.parseCommit(oldObjectId));
+        } else {
+          markUninteresting(git, branch, rw, oldObjectId);
+        }
+      } catch (Exception e) {
+        log.error(e.getMessage(), e);
+        return;
       }
-    } catch (Exception e) {
-      log.error(e.getMessage(), e);
-      return;
-    }
 
-    if (monitor == null) {
-      monitor = NullProgressMonitor.INSTANCE;
-    }
+      if (monitor == null) {
+        monitor = NullProgressMonitor.INSTANCE;
+      }
 
-    try {
       for (RevCommit c : rw) {
         ObjectId content = createNoteContent(loadPatchSet(c, branch));
         if (content != null) {
@@ -156,8 +155,6 @@
           getMessage().append("* ").append(c.getShortMessage()).append("\n");
         }
       }
-    } finally {
-      rw.close();
     }
   }