Redefine commit_* to NOT unlock

Update all callers to call unlock_foo after every commit_foo. Add
tests to check that commit_* do not unlock. Fix some tests that were
meaningless. Change commit_* implementations to not unlock.

This allows us to commit, then continue, e.g., for forget or backup
checkpoints. The checkpoints still unlock and re-lock, however, due to
tests failing otherwise, indicating a problem elswewhere.

6 files changed, 105 insertions, 33 deletions

diff --git a/obnamlib/delegator.py b/obnamlib/delegator.py
index e1b2a220..b1240fce 100644
--- a/obnamlib/delegator.py
+++ b/obnamlib/delegator.py
@@ -94,7 +94,6 @@ class RepositoryDelegator(obnamlib.RepositoryInterface):
     def commit_client_list(self):
         self._require_we_got_client_list_lock()
         self._client_list.commit()
-        self.unlock_client_list()
 
     def got_client_list_lock(self):
         dirname = self._client_list.get_dirname()
@@ -163,7 +162,6 @@ class RepositoryDelegator(obnamlib.RepositoryInterface):
         self._require_got_client_lock(client_name)
         client = self._lookup_client(client_name)
         client.commit()
-        self.unlock_client(client_name)
 
     def got_client_lock(self, client_name):
         client = self._lookup_client(client_name)
@@ -310,7 +308,6 @@ class RepositoryDelegator(obnamlib.RepositoryInterface):
     def commit_chunk_indexes(self):
         self._require_we_got_chunk_indexes_lock()
         self._chunk_indexes.commit()
-        self.unlock_chunk_indexes()
 
     def got_chunk_indexes_lock(self):
         dirname = self._chunk_indexes.get_dirname()
diff --git a/obnamlib/fmt_6/repo_fmt_6.py b/obnamlib/fmt_6/repo_fmt_6.py
index f80a12df..9424df05 100644
--- a/obnamlib/fmt_6/repo_fmt_6.py
+++ b/obnamlib/fmt_6/repo_fmt_6.py
@@ -135,12 +135,12 @@ class RepositoryFormat6(obnamlib.RepositoryInterface):
 
     def commit_client_list(self):
         tracing.trace('committing client list')
+        self._require_client_list_lock()
         for client_name in self._added_clients:
             self.hooks.call(
                 'repository-add-client', self, client_name)
         self._added_clients = []
         self._client_list.commit()
-        self._raw_unlock_client_list()
 
     def got_client_list_lock(self):
         return self._lockmgr.got_lock('.')
@@ -339,8 +339,6 @@ class RepositoryFormat6(obnamlib.RepositoryInterface):
         if need_to_commit:
             open_client_info.client.commit()
 
-        self._raw_unlock_client(client_name)
-
     def _remove_chunks_from_removed_generations(
             self, client_name, remove_gen_nos):
 
@@ -750,7 +748,6 @@ class RepositoryFormat6(obnamlib.RepositoryInterface):
         self._require_chunk_indexes_lock()
         self._lockmgr.unlock(self._chunk_index_dirs_to_lock())
         self._setup_chunk_indexes()
-        self._reset_unused_chunks()
 
     def lock_chunk_indexes(self):
         tracing.trace('locking chunk indexes')
@@ -759,6 +756,7 @@ class RepositoryFormat6(obnamlib.RepositoryInterface):
     def unlock_chunk_indexes(self):
         tracing.trace('unlocking chunk indexes')
         self._raw_unlock_chunk_indexes()
+        self._reset_unused_chunks()
 
     def got_chunk_indexes_lock(self):
         return all(
@@ -775,7 +773,6 @@ class RepositoryFormat6(obnamlib.RepositoryInterface):
         self._require_chunk_indexes_lock()
         self._chunklist.commit()
         self._chunksums.commit()
-        self._raw_unlock_chunk_indexes()
 
     def prepare_chunk_for_indexes(self, data):
         return self._checksum(data)
diff --git a/obnamlib/plugins/backup_plugin.py b/obnamlib/plugins/backup_plugin.py
index 1f1aaff8..92b66c03 100644
--- a/obnamlib/plugins/backup_plugin.py
+++ b/obnamlib/plugins/backup_plugin.py
@@ -403,9 +403,11 @@ class BackupPlugin(obnamlib.ObnamPlugin):
         self.progress.what(prefix + 'committing client')
         self.repo.flush_chunks()
         self.repo.commit_client(self.client_name)
+        self.repo.unlock_client(self.client_name)
 
         self.progress.what(prefix + 'committing shared B-trees')
         self.repo.commit_chunk_indexes()
+        self.repo.unlock_chunk_indexes()
 
     def should_remove_checkpoints(self):
         return (not self.progress.errors and
@@ -423,9 +425,11 @@ class BackupPlugin(obnamlib.ObnamPlugin):
 
         self.progress.what(prefix + ': committing client')
         self.repo.commit_client(self.client_name)
+        self.repo.unlock_client(self.client_name)
 
         self.progress.what(prefix + ': commiting shared B-trees')
         self.repo.commit_chunk_indexes()
+        self.repo.unlock_chunk_indexes()
 
         self.progress.what(prefix + ': removing unused chunks')
         self.repo.remove_unused_chunks()
@@ -487,6 +491,7 @@ class BackupPlugin(obnamlib.ObnamPlugin):
             tracing.trace('client list after adding: %s' %
                           self.repo.get_client_names())
         self.repo.commit_client_list()
+        self.repo.unlock_client_list()
         self.repo = self.app.get_repository_object(repofs=self.repo.get_fs())
 
     def backup_roots(self, roots):
@@ -656,15 +661,16 @@ class BackupPlugin(obnamlib.ObnamPlugin):
                 obnamlib.REPO_GENERATION_IS_CHECKPOINT, 1)
             self.repo.flush_chunks()
             self.repo.commit_client(self.client_name)
+            self.repo.unlock_client(self.client_name)
             self.progress.what('making checkpoint: committing shared B-trees')
             self.repo.commit_chunk_indexes()
+            self.repo.unlock_chunk_indexes()
             self.last_checkpoint = self.repo.get_fs().bytes_written
             self.progress.what('making checkpoint: re-opening repository')
             self.repo = self.app.get_repository_object(
                 repofs=self.repo.get_fs())
-            self.progress.what('making checkpoint: locking client')
-            self.repo.lock_client(self.client_name)
             self.progress.what('making checkpoint: starting a new generation')
+            self.repo.lock_client(self.client_name)
             self.new_generation = self.repo.create_generation(
                 self.client_name)
             self.app.dump_memory_profile('at end of checkpoint')
diff --git a/obnamlib/plugins/encryption_plugin.py b/obnamlib/plugins/encryption_plugin.py
index 53988199..2ff62a69 100644
--- a/obnamlib/plugins/encryption_plugin.py
+++ b/obnamlib/plugins/encryption_plugin.py
@@ -287,3 +287,4 @@ class EncryptionPlugin(obnamlib.ObnamPlugin):
             logging.info('removing client %s' % client_name)
             repo.remove_client(client_name)
         repo.commit_client_list()
+        repo.unlock_client_list()
diff --git a/obnamlib/plugins/forget_plugin.py b/obnamlib/plugins/forget_plugin.py
index 81b43453..a698d177 100644
--- a/obnamlib/plugins/forget_plugin.py
+++ b/obnamlib/plugins/forget_plugin.py
@@ -87,8 +87,8 @@ class ForgetPlugin(obnamlib.ObnamPlugin):
         # Commit or unlock everything.
         self.repo.commit_client(client_name)
         self.repo.commit_chunk_indexes()
-        self.repo.remove_unused_chunks()
         self.repo.unlock_everything()
+        self.repo.remove_unused_chunks()
         self.app.dump_memory_profile('after committing')
 
         self.repo.close()
diff --git a/obnamlib/repo_interface.py b/obnamlib/repo_interface.py
index 4cb05d44..7c00a0fe 100644
--- a/obnamlib/repo_interface.py
+++ b/obnamlib/repo_interface.py
@@ -389,7 +389,13 @@ class RepositoryInterface(object):
             raise
 
     def unlock_everything(self):
-        '''Unock every part of the repository to which we hold a lock.'''
+        '''Unock every part of the repository to which we hold a lock.
+
+        This also discards any changes made to each part made after
+        the latest lock or commit of that part.
+
+        '''
+
         if self.got_client_list_lock():
             self.unlock_client_list()
 
@@ -411,11 +417,21 @@ class RepositoryInterface(object):
         raise NotImplementedError()
 
     def commit_client_list(self):
-        '''Commit changes to client list and unlock it.'''
+        '''Commit changes to client list and DO NOT unlock it.
+
+        Note that the caller MUST also call unlock_client_list.
+        Otherwise the client list remains locked.
+
+        '''
         raise NotImplementedError()
 
     def unlock_client_list(self):
-        '''Forget changes to client list and unlock it.'''
+        '''Forget changes to client list and unlock it.
+
+        The changes that are forgotten are those made after the latest
+        lock_client_list or commit_client_list.
+
+        '''
         raise NotImplementedError()
 
     def got_client_list_lock(self):
@@ -479,11 +495,21 @@ class RepositoryInterface(object):
         raise NotImplementedError()
 
     def commit_client(self, client_name):
-        '''Commit changes to client and unlock it.'''
+        '''Commit changes to client and DO NOT unlock it.
+
+        Note that the caller MUST also call unlock_client. Otherwise
+        the client list remains locked.
+
+        '''
         raise NotImplementedError()
 
     def unlock_client(self, client_name):
-        '''Forget changes to client and unlock it.'''
+        '''Forget changes to client and unlock it.
+
+        The changes that are forgotten are those made after the latest
+        lock_client or commit_client.
+
+        '''
         raise NotImplementedError()
 
     def got_client_lock(self, client_name):
@@ -754,7 +780,12 @@ class RepositoryInterface(object):
         raise NotImplementedError()
 
     def unlock_chunk_indexes(self):
-        '''Unlocks chunk indexes without committing them.'''
+        '''Forget any changes to and and unlock chunk indexes.
+
+        The changes that are forgotten are those made after the latest
+        lock_chunk_indexes or commit_chunk_indexes.
+
+        '''
         raise NotImplementedError()
 
     def got_chunk_indexes_lock(self):
@@ -766,7 +797,12 @@ class RepositoryInterface(object):
         raise NotImplementedError()
 
     def commit_chunk_indexes(self):
-        '''Commit changes to chunk indexes.'''
+        '''Commit changes to chunk indexes and DO NOT unlock it.
+
+        Note that the caller MUST also call unlock_chunk_indexes.
+        Otherwise the client list remains locked.
+
+        '''
         raise NotImplementedError()
 
     def prepare_chunk_for_indexes(self, data):
@@ -948,6 +984,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.lock_client_list()
         self.repo.add_client('foo')
         self.repo.commit_client_list()
+        self.repo.unlock_client_list()
 
         self.repo.lock_client_list()
         self.repo.rename_client('foo', 'bar')
@@ -982,6 +1019,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.add_client('foo')
         self.repo.add_client('bar')
         self.repo.commit_client_list()
+        self.repo.unlock_client_list()
 
         self.repo.lock_client_list()
         self.assertRaises(
@@ -1006,6 +1044,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.lock_client_list()
         self.repo.add_client('foo')
         self.repo.commit_client_list()
+        self.repo.unlock_client_list()
         self.assertRaises(
             obnamlib.RepositoryClientListNotLocked,
             self.repo.rename_client, 'foo', 'bar')
@@ -1029,6 +1068,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.lock_client_list()
         self.repo.add_client('foo')
         self.repo.commit_client_list()
+        self.repo.unlock_client_list()
 
         self.repo.lock_client_list()
         self.repo.rename_client('foo', 'bar')
@@ -1042,6 +1082,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.lock_client_list()
         self.repo.add_client('foo')
         self.repo.commit_client_list()
+        self.repo.unlock_client_list()
 
         self.repo.lock_client_list()
         self.repo.remove_client('foo')
@@ -1054,6 +1095,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.lock_client_list()
         self.repo.add_client('foo')
         self.repo.commit_client_list()
+        self.repo.unlock_client_list()
         self.assertEqual(self.repo.get_client_names(), ['foo'])
 
     def test_committing_client_list_renames_client(self):
@@ -1062,10 +1104,12 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.lock_client_list()
         self.repo.add_client('foo')
         self.repo.commit_client_list()
+        self.repo.unlock_client_list()
 
         self.repo.lock_client_list()
         self.repo.rename_client('foo', 'bar')
         self.repo.commit_client_list()
+        self.repo.unlock_client_list()
 
         self.assertEqual(self.repo.get_client_names(), ['bar'])
 
@@ -1075,30 +1119,26 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.lock_client_list()
         self.repo.add_client('foo')
         self.repo.commit_client_list()
+        self.repo.unlock_client_list()
 
         self.repo.lock_client_list()
         self.repo.remove_client('foo')
         self.repo.commit_client_list()
+        self.repo.unlock_client_list()
 
         self.assertEqual(self.repo.get_client_names(), [])
 
-    def test_commiting_client_list_removes_lock(self):
+    def test_commiting_client_list_does_NOT_remove_lock(self):
         self.repo.init_repo()
-
         self.repo.lock_client_list()
         self.repo.commit_client_list()
-
-        self.repo.lock_client_list()
-        self.assertEqual(self.repo.get_client_names(), [])
+        self.assertTrue(self.repo.got_client_list_lock())
 
     def test_unlocking_client_list_removes_lock(self):
         self.repo.init_repo()
-
         self.repo.lock_client_list()
         self.repo.unlock_client_list()
-
-        self.repo.lock_client_list()
-        self.assertEqual(self.repo.get_client_names(), [])
+        self.assertFalse(self.repo.got_client_list_lock())
 
     def test_locking_client_list_twice_fails(self):
         self.repo.init_repo()
@@ -1167,12 +1207,14 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.lock_client_list()
         self.repo.add_client('fooclient')
         self.repo.commit_client_list()
+        self.repo.unlock_client_list()
 
     def setup_two_clients(self):
         self.repo.lock_client_list()
         self.repo.add_client('fooclient')
         self.repo.add_client('barclient')
         self.repo.commit_client_list()
+        self.repo.unlock_client_list()
 
     def test_have_not_got_client_lock_initially(self):
         self.setup_client()
@@ -1235,17 +1277,17 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
             obnamlib.RepositoryClientDoesNotExist,
             self.repo.commit_client, 'notexist')
 
-    def test_unlocking_client_removes_lock(self):
+    def test_committing_client_does_NOT_remove_lock(self):
         self.setup_client()
         self.repo.lock_client('fooclient')
-        self.repo.unlock_client('fooclient')
-        self.assertEqual(self.repo.lock_client('fooclient'), None)
+        self.repo.commit_client('fooclient')
+        self.assertTrue(self.repo.got_client_lock('fooclient'))
 
-    def test_committing_client_removes_lock(self):
+    def test_unlocking_client_removes_lock(self):
         self.setup_client()
         self.repo.lock_client('fooclient')
-        self.repo.commit_client('fooclient')
-        self.assertEqual(self.repo.lock_client('fooclient'), None)
+        self.repo.unlock_client('fooclient')
+        self.assertFalse(self.repo.got_client_lock('fooclient'))
 
     def test_has_list_of_allowed_client_keys(self):
         self.assertEqual(type(self.repo.get_allowed_client_keys()), list)
@@ -1301,6 +1343,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
             value = self.repo.get_client_key(
                 'fooclient', obnamlib.REPO_CLIENT_TEST_KEY)
             self.repo.commit_client('fooclient')
+            self.repo.unlock_client('fooclient')
             self.assertEqual(value, 'bar')
 
     def test_unlocking_client_undoes_key_changes(self):
@@ -1360,6 +1403,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.lock_client('fooclient')
         new_id = self.repo.create_generation('fooclient')
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
         self.assertEqual(
             self.repo.get_client_generation_ids('fooclient'),
             [new_id])
@@ -1417,6 +1461,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         if self.generation_test_key_is_allowed():
             gen_id = self.create_generation()
             self.repo.commit_client('fooclient')
+            self.repo.unlock_client('fooclient')
             self.assertRaises(
                 obnamlib.RepositoryClientNotLocked,
                 self.repo.set_generation_key,
@@ -1430,6 +1475,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
             value = self.repo.get_generation_key(
                 gen_id, obnamlib.REPO_GENERATION_TEST_KEY)
             self.repo.commit_client('fooclient')
+            self.repo.unlock_client('fooclient')
             self.assertEqual(value, 'bar')
 
     def test_removes_unfinished_generation(self):
@@ -1440,6 +1486,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
     def test_removes_finished_generation(self):
         gen_id = self.create_generation()
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
         self.repo.lock_client('fooclient')
         self.repo.remove_generation(gen_id)
         self.assertEqual(self.repo.get_client_generation_ids('fooclient'), [])
@@ -1454,6 +1501,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
     def test_removing_generation_without_client_lock_fails(self):
         gen_id = self.create_generation()
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
         self.assertRaises(
             obnamlib.RepositoryClientNotLocked,
             self.repo.remove_generation, gen_id)
@@ -1461,6 +1509,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
     def test_unlocking_client_forgets_generation_removal(self):
         gen_id = self.create_generation()
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
         self.repo.lock_client('fooclient')
         self.repo.remove_generation(gen_id)
         self.repo.unlock_client('fooclient')
@@ -1472,6 +1521,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         gen_id = self.create_generation()
         self.repo.remove_generation(gen_id)
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
         self.assertEqual(self.repo.get_client_generation_ids('fooclient'), [])
 
     def test_empty_generation_uses_no_chunk_ids(self):
@@ -1502,6 +1552,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         # one.
         gen_id = self.create_generation()
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
         self.repo.lock_client('fooclient')
         gen_id_2 = self.repo.create_generation('fooclient')
         genspec = self.repo.make_generation_spec(gen_id)
@@ -1533,6 +1584,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.set_file_key(
             gen_id, '/foo/bar', obnamlib.REPO_FILE_MODE, stat.S_IFREG)
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
         self.assertTrue(self.repo.file_exists(gen_id, '/foo/bar'))
 
     def test_creating_generation_clones_previous_one(self):
@@ -1541,6 +1593,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.set_file_key(
             gen_id, '/foo/bar', obnamlib.REPO_FILE_MODE, stat.S_IFREG)
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
 
         self.repo.lock_client('fooclient')
         gen_id_2 = self.repo.create_generation('fooclient')
@@ -1558,6 +1611,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.set_file_key(
             gen_id, '/foo/bar', obnamlib.REPO_FILE_MODE, stat.S_IFREG)
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
 
         self.repo.lock_client('fooclient')
         gen_id_2 = self.repo.create_generation('fooclient')
@@ -1572,12 +1626,14 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.set_file_key(
             gen_id, '/foo/bar', obnamlib.REPO_FILE_MODE, stat.S_IFREG)
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
 
         self.repo.lock_client('fooclient')
         gen_id_2 = self.repo.create_generation('fooclient')
         self.assertTrue(self.repo.file_exists(gen_id_2, '/foo/bar'))
         self.repo.remove_file(gen_id_2, '/foo/bar')
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
 
         self.assertTrue(self.repo.file_exists(gen_id, '/foo/bar'))
         self.assertFalse(self.repo.file_exists(gen_id_2, '/foo/bar'))
@@ -1758,6 +1814,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.set_file_key(
             gen_id, '/foo/bar', obnamlib.REPO_FILE_TEST_KEY, 'yoyo')
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
         value = self.repo.get_file_key(
             gen_id, '/foo/bar', obnamlib.REPO_FILE_TEST_KEY)
         self.assertEqual(value, 'yoyo')
@@ -1770,12 +1827,14 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         self.repo.set_file_key(
             gen_id, '/foo/bar', obnamlib.REPO_FILE_TEST_KEY, 'first')
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
 
         self.repo.lock_client('fooclient')
         gen_id_2 = self.repo.create_generation('fooclient')
         self.repo.set_file_key(
             gen_id_2, '/foo/bar', obnamlib.REPO_FILE_TEST_KEY, 'second')
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
 
         value = self.repo.get_file_key(
             gen_id, '/foo/bar', obnamlib.REPO_FILE_TEST_KEY)
@@ -1866,6 +1925,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
             gen_id, '/foo/bar', obnamlib.REPO_FILE_MODE, stat.S_IFREG)
         self.repo.append_file_chunk_id(gen_id, '/foo/bar', 1)
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
 
         self.repo.lock_client('fooclient')
         gen_id_2 = self.repo.create_generation('fooclient')
@@ -1886,6 +1946,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
             gen_id, '/foo/bar', obnamlib.REPO_FILE_MODE, stat.S_IFREG)
         self.repo.append_file_chunk_id(gen_id, '/foo/bar', 1)
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
 
         self.repo.lock_client('fooclient')
         gen_id_2 = self.repo.create_generation('fooclient')
@@ -1895,6 +1956,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
             [1, 2])
 
         self.repo.commit_client('fooclient')
+        self.repo.unlock_client('fooclient')
         self.assertEqual(
             self.repo.get_file_chunk_ids(gen_id, '/foo/bar'),
             [1])
@@ -2043,6 +2105,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         token = self.repo.prepare_chunk_for_indexes('foochunk')
         self.repo.put_chunk_into_indexes(chunk_id, token, 'fooclient')
         self.repo.commit_chunk_indexes()
+        self.repo.unlock_chunk_indexes()
         self.assertRaises(
             obnamlib.RepositoryChunkIndexesNotLocked,
             self.repo.remove_chunk_from_indexes, chunk_id, 'fooclient')
@@ -2054,6 +2117,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         token = self.repo.prepare_chunk_for_indexes('foochunk')
         self.repo.put_chunk_into_indexes(chunk_id, token, 'fooclient')
         self.repo.commit_chunk_indexes()
+        self.repo.unlock_chunk_indexes()
         self.assertRaises(
             obnamlib.RepositoryChunkIndexesNotLocked,
             self.repo.remove_chunk_from_indexes_for_all_clients, chunk_id)
@@ -2076,9 +2140,15 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         token = self.repo.prepare_chunk_for_indexes('foochunk')
         self.repo.put_chunk_into_indexes(chunk_id, token, 'fooclient')
         self.repo.commit_chunk_indexes()
+        self.repo.unlock_chunk_indexes()
         self.assertEqual(
             self.repo.find_chunk_ids_by_content('foochunk'), [chunk_id])
 
+    def test_committing_does_NOT_remove_chunk_indexes_lock(self):
+        self.repo.lock_chunk_indexes()
+        self.repo.commit_chunk_indexes()
+        self.assertTrue(self.repo.got_chunk_indexes_lock())
+
     def test_locking_chunk_indexes_twice_fails(self):
         self.repo.lock_chunk_indexes()
         self.assertRaises(
@@ -2102,6 +2172,7 @@ class RepositoryInterfaceTests(unittest.TestCase):  # pragma: no cover
         token = self.repo.prepare_chunk_for_indexes('foochunk')
         self.repo.put_chunk_into_indexes(chunk_id, token, 'fooclient')
         self.repo.commit_chunk_indexes()
+        self.repo.unlock_chunk_indexes()
         ret = self.repo.validate_chunk_content(chunk_id)
         self.assertTrue(ret is True or ret is None)