Skip to content

Commit

Permalink
We should actually be releasing new_instance() storages, not closing …
Browse files Browse the repository at this point in the history
…them. closing can impact other MVCC storages.
  • Loading branch information
jamadden committed Jun 24, 2019
1 parent d03dd97 commit ce2dd17
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 6 deletions.
6 changes: 5 additions & 1 deletion src/ZODB/DB.py
Original file line number Diff line number Diff line change
Expand Up @@ -1066,7 +1066,11 @@ def abort(self, transaction):

def close(self):
if self._storage is not None:
self._storage.close()
# We actually want to release the storage we've created,
# not close it. releasing it frees external resources
# dedicated to this instance, closing might make permanent
# changes that affect other instances.
self._storage.release()
self._storage = None

def tpc_begin(self, transaction):
Expand Down
20 changes: 15 additions & 5 deletions src/ZODB/tests/testDB.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ def _makeOne(self):
class MockStorage(object):
instance_count = 0
close_count = 0
release_count = 0
begin_count = 0
abort_count = 0

Expand All @@ -132,6 +133,9 @@ def tpc_begin(self, tx):
def close(self):
self.close_count += 1

def release(self):
self.release_count += 1

def sortKey(self):
return 'MockStorage'

Expand Down Expand Up @@ -165,11 +169,13 @@ def test_close_many(self):
undo.close()
# Not open, didn't go through
self.assertEqual(0, undo._db.storage.close_count)
self.assertEqual(0, undo._db.storage.release_count)

undo.tpc_begin(transaction.get())
undo.close()
undo.close()
self.assertEqual(1, undo._db.storage.close_count)
self.assertEqual(0, undo._db.storage.close_count)
self.assertEqual(1, undo._db.storage.release_count)
self.assertIsNone(undo._storage)

def test_sortKey(self):
Expand All @@ -187,28 +193,32 @@ def test_tpc_abort_closes(self):
undo.tpc_begin(transaction.get())
undo._db.storage.tpc_abort = lambda tx: None
undo.tpc_abort(transaction.get())
self.assertEqual(1, undo._db.storage.close_count)
self.assertEqual(0, undo._db.storage.close_count)
self.assertEqual(1, undo._db.storage.release_count)

def test_tpc_abort_closes_on_exception(self):
undo = self._makeOne()
undo.tpc_begin(transaction.get())
with self.assertRaises(AttributeError):
undo.tpc_abort(transaction.get())
self.assertEqual(1, undo._db.storage.close_count)
self.assertEqual(0, undo._db.storage.close_count)
self.assertEqual(1, undo._db.storage.release_count)

def test_tpc_finish_closes(self):
undo = self._makeOne()
undo.tpc_begin(transaction.get())
undo._db.storage.tpc_finish = lambda tx: None
undo.tpc_finish(transaction.get())
self.assertEqual(1, undo._db.storage.close_count)
self.assertEqual(0, undo._db.storage.close_count)
self.assertEqual(1, undo._db.storage.release_count)

def test_tpc_finish_closes_on_exception(self):
undo = self._makeOne()
undo.tpc_begin(transaction.get())
with self.assertRaises(AttributeError):
undo.tpc_finish(transaction.get())
self.assertEqual(1, undo._db.storage.close_count)
self.assertEqual(0, undo._db.storage.close_count)
self.assertEqual(1, undo._db.storage.release_count)


def test_invalidateCache():
Expand Down

0 comments on commit ce2dd17

Please sign in to comment.