Commit 5a5ed2c7 authored by Kirill Smelkov's avatar Kirill Smelkov

tests: Force-close ZODB connections in teardown, that testing code forgot to explicitly close

If a test forgets to explicitly close ZODB connection it was using, this
connection stays alive in transaction synchronizers (it is a weakset),
and continues to be used on e.g. transaction.commit() when all
synchronizers are invoked. This could lead to crashes like below when
underlying ZODB storage was closed by test module teardown and testing
moved on to another test module:

    $ WENDELIN_CORE_TEST_DB="<neo>" py.test  bigfile/tests/test_filezodb.py::test_bigfile_zblk1_zdata_reuse lib/tests/test_zodb.py
    ======= test session starts ========
    platform linux2 -- Python 2.7.14+, pytest-3.5.0, py-1.5.3, pluggy-0.6.0
    rootdir: /home/kirr/src/wendelin/wendelin.core, inifile:
    collected 2 items

    bigfile/tests/test_filezodb.py .                                     [ 50%]
    lib/tests/test_zodb.py F                                             [100%]

    ______ test_deactivate_btree _______

        def test_deactivate_btree():
            root = dbopen()
            # init btree with many leaf nodes
            leafv = []
            root['btree'] = B = IOBTree()
            for i in range(10000):
                B[i] = xi = XInt(i)
                leafv.append(xi)
    >       transaction.commit()

    lib/tests/test_zodb.py:56:
    _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
    ../venv/z5/local/lib/python2.7/site-packages/transaction/_manager.py:131: in commit
        return self.get().commit()
    ../venv/z5/local/lib/python2.7/site-packages/transaction/_transaction.py:316: in commit
        self._synchronizers.map(lambda s: s.afterCompletion(self))
    ../venv/z5/local/lib/python2.7/site-packages/transaction/weakset.py:62: in map
        f(elt)
    ../venv/z5/local/lib/python2.7/site-packages/transaction/_transaction.py:316: in <lambda>
        self._synchronizers.map(lambda s: s.afterCompletion(self))
    ../venv/z5/local/lib/python2.7/site-packages/ZODB/Connection.py:757: in afterCompletion
        self.newTransaction(transaction, False)
    ../venv/z5/local/lib/python2.7/site-packages/ZODB/Connection.py:737: in newTransaction
        invalidated = self._storage.poll_invalidations()
    ../venv/z5/local/lib/python2.7/site-packages/ZODB/mvccadapter.py:131: in poll_invalidations
        self._start = p64(u64(self._storage.lastTransaction()) + 1)
    _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

    self = <neo.client.Storage.Storage object at 0x7ffa1be8d410>

        def lastTransaction(self):
            # Used in ZODB unit tests
    >       return self.app.last_tid
    E       AttributeError: 'NoneType' object has no attribute 'last_tid'

    ../../neo/src/lab.nexedi.com/kirr/neo/neo/client/Storage.py:181: AttributeError

where NEO's Storage.app is None because the storage was closed.

----

To avoid such kind of failures make sure TestDB.teardown() always closes
all ZODB connections that were ever opened via TestDB.dbopen().

Add a warning about such force-closing with information about corresponding
connection and code place that created it, so that it is easy to
understand which test needs a fix.

/suggested-by @jm
parent 72df6eb6
...@@ -18,15 +18,18 @@ ...@@ -18,15 +18,18 @@
# See COPYING file for full licensing terms. # See COPYING file for full licensing terms.
# See https://www.nexedi.com/licensing for rationale and options. # See https://www.nexedi.com/licensing for rationale and options.
from __future__ import print_function
from wendelin.lib.zodb import dbstoropen from wendelin.lib.zodb import dbstoropen
from zlib import adler32 from zlib import adler32
from struct import pack from struct import pack
from tempfile import mkdtemp from tempfile import mkdtemp
from shutil import rmtree from shutil import rmtree
from ZODB import DB from ZODB import DB
import weakref, traceback
import codecs import codecs
import math import math
import os import os, sys
import pkg_resources import pkg_resources
# hashlib-like interface to adler32 # hashlib-like interface to adler32
...@@ -171,7 +174,7 @@ def ffadler32_bysize(size): return _ffadler32_byorder [ilog2_exact(size)] ...@@ -171,7 +174,7 @@ def ffadler32_bysize(size): return _ffadler32_byorder [ilog2_exact(size)]
class TestDB_Base(object): class TestDB_Base(object):
def setup(self): def setup(self):
raise NotImplementedError() raise NotImplementedError()
def teardown(self): def _teardown(self):
raise NotImplementedError() raise NotImplementedError()
def getZODBStorage(self): def getZODBStorage(self):
raise NotImplementedError() raise NotImplementedError()
...@@ -182,6 +185,7 @@ class TestDB_Base(object): ...@@ -182,6 +185,7 @@ class TestDB_Base(object):
stor = self.getZODBStorage() stor = self.getZODBStorage()
db = DB(stor) db = DB(stor)
conn = db.open() conn = db.open()
self.connv.append( (weakref.ref(conn), ''.join(traceback.format_stack())) )
root = conn.root() root = conn.root()
return root return root
...@@ -189,6 +193,35 @@ class TestDB_Base(object): ...@@ -189,6 +193,35 @@ class TestDB_Base(object):
def __init__(self, dburi): def __init__(self, dburi):
self.dburi = dburi self.dburi = dburi
# remember all database connections that were born via dbopen.
#
# if test code is not careful enough to close them - we'll close in
# teardown to separate failures in between modules.
#
# we don't use strong references, because part of transaction/ZODB
# keeps weak references to connections.
self.connv = [] # [] of (weakref(conn), traceback)
def teardown(self):
# close connections that test code forgot to close
for connref, tb in self.connv:
conn = connref()
if conn is None:
continue
if not conn.opened:
continue # still alive, but closed
print("W: testdb: teardown: %s left not closed by test code"
"; opened by:\n%s" % (conn, tb), file=sys.stderr)
db = conn.db()
stor = db.storage
conn.close()
# DB.close() should close underlying storage and is noop when
# called the second time.
db.close()
self._teardown()
# FileStorage for tests # FileStorage for tests
...@@ -197,7 +230,7 @@ class TestDB_FileStorage(TestDB_Base): ...@@ -197,7 +230,7 @@ class TestDB_FileStorage(TestDB_Base):
def setup(self): def setup(self):
self.tmpd = mkdtemp('', 'testdb_fs.') self.tmpd = mkdtemp('', 'testdb_fs.')
def teardown(self): def _teardown(self):
rmtree(self.tmpd) rmtree(self.tmpd)
def getZODBStorage(self): def getZODBStorage(self):
...@@ -234,7 +267,7 @@ class TestDB_ZEO(TestDB_Base): ...@@ -234,7 +267,7 @@ class TestDB_ZEO(TestDB_Base):
else: else:
self.addr, self.adminaddr, self.pid, self.path = _ self.addr, self.adminaddr, self.pid, self.path = _
def teardown(self): def _teardown(self):
if self.z5: if self.z5:
self.stop() self.stop()
else: else:
...@@ -258,7 +291,7 @@ class TestDB_NEO(TestDB_Base): ...@@ -258,7 +291,7 @@ class TestDB_NEO(TestDB_Base):
self.cluster.start() self.cluster.start()
self.cluster.expectClusterRunning() self.cluster.expectClusterRunning()
def teardown(self): def _teardown(self):
self.cluster.stop() self.cluster.stop()
def getZODBStorage(self): def getZODBStorage(self):
...@@ -270,8 +303,8 @@ class TestDB_NEO(TestDB_Base): ...@@ -270,8 +303,8 @@ class TestDB_NEO(TestDB_Base):
class TestDB_External(TestDB_Base): class TestDB_External(TestDB_Base):
# we do not create/destroy it - the database managed not by us # we do not create/destroy it - the database managed not by us
def setup(self): pass def setup(self): pass
def teardown(self): pass def _teardown(self): pass
def getZODBStorage(self): def getZODBStorage(self):
return dbstoropen(self.dburi) return dbstoropen(self.dburi)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment