From 7eba4815d2a0e8318cb90047aae9fe6c7433d9b5 Mon Sep 17 00:00:00 2001
From: Dave Borowitz <dborowitz@google.com>
Date: Mon, 28 Dec 2015 13:58:03 -0800
Subject: [PATCH] AbstractChangeUpdate: Don't prematurely close Repository

Using MetaDataUpdate in a try-with-resources block closes the repo as
soon as the BatchMetaDataUpdate is opened, which is not what we want.
In the traditional RepositoryCache case this doesn't hurt, since we
maintain a nonzero refcount for open repos so the close is a no-op,
but this is not always the case.

Instead, don't close the MetaDataUpdate implicitly, but rather
explicitly close it from the close method of the enclosing
BatchMetaDataUpdate.

Change-Id: I40871efd38d11375367a2909cf8eff4443e68f20
---
 .../gerrit/server/notedb/AbstractChangeUpdate.java   | 12 +++++-------
 .../google/gerrit/server/notedb/ChangeNotesTest.java |  5 ++++-
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
index baff009824..61c9aa40c1 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java
@@ -117,13 +117,11 @@ public abstract class AbstractChangeUpdate extends VersionedMetaData {
       throws IOException {
     if (migration.writeChanges()) {
       load();
-      try (MetaDataUpdate md =
-            updateFactory.create(getProjectName(),
-                repoManager.openMetadataRepository(getProjectName()), getUser(),
-                bru)) {
-        md.setAllowEmpty(true);
-        return super.openUpdate(md);
-      }
+      Project.NameKey p = getProjectName();
+      MetaDataUpdate md = updateFactory.create(
+          p, repoManager.openMetadataRepository(p), getUser(), bru);
+      md.setAllowEmpty(true);
+      return super.openUpdate(md);
     }
     return new BatchMetaDataUpdate() {
       @Override
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
index d932924533..04b51fd715 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -408,7 +408,6 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
   @Test
   public void topicChangeNotes() throws Exception {
     Change c = newChange();
-    ChangeUpdate update = newUpdate(c, changeOwner);
 
     // initially topic is not set
     ChangeNotes notes = newNotes(c);
@@ -417,12 +416,14 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
 
     // set topic
     String topic = "myTopic";
+    ChangeUpdate update = newUpdate(c, changeOwner);
     update.setTopic(topic);
     update.commit();
     notes = newNotes(c);
     assertThat(notes.getChange().getTopic()).isEqualTo(topic);
 
     // clear topic by setting empty string
+    update = newUpdate(c, changeOwner);
     update.setTopic("");
     update.commit();
     notes = newNotes(c);
@@ -430,12 +431,14 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
 
     // set other topic
     topic = "otherTopic";
+    update = newUpdate(c, changeOwner);
     update.setTopic(topic);
     update.commit();
     notes = newNotes(c);
     assertThat(notes.getChange().getTopic()).isEqualTo(topic);
 
     // clear topic by setting null
+    update = newUpdate(c, changeOwner);
     update.setTopic(null);
     update.commit();
     notes = newNotes(c);