Skip to content

Commit

Permalink
*: Fix many ResourceWarnings
Browse files Browse the repository at this point in the history
--------
kirr:

Currently running ZEO tests emits lots of ResourceWarning, for example

    test_ZEO_DB_convenience_error (ZEO.tests.testZEO.Test_convenience_functions).../lib/python3.9/site-packages/mock/mock.py:2119: ResourceWarning: unclosed file <_io.FileIO name=4 mode='rb+' closefd=True>
    test_ZEO_connection_convenience_ok (ZEO.tests.testZEO.Test_convenience_functions).../lib/python3.9/site-packages/mock/mock.py:2072: ResourceWarning: unclosed file <_io.FileIO name=4 mode='rb+' closefd=True>
    some_basic_locking_tests (ZEO.tests.testZEO2)/usr/lib/python3.9/logging/__init__.py:1063: ResourceWarning: unclosed <socket.socket fd=10, family=AddressFamily.AF_UNIX, type=SocketKind.SOCK_STREAM, proto=0, laddr=x>
    ...

The patch "asyncio.client: Fix bugs in closing and connecting logic"
fixed closing/connecting asyncio bugs that were indicated by some
ResourceWarnings. However there are more ResourceWarnings that are
correctly emitted due to e.g. not closed IO or uncompleted transactions.

-> Fix them.

Extracted from #195
More details at: #195 (comment)
  • Loading branch information
d-maurer authored and navytux committed Dec 30, 2022
1 parent f09b260 commit 9f586d5
Show file tree
Hide file tree
Showing 9 changed files with 29 additions and 3 deletions.
4 changes: 3 additions & 1 deletion src/ZEO/StorageServer.py
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,9 @@ def iterator_record_next(self, iid):

def iterator_gc(self, iids):
for iid in iids:
self._iterators.pop(iid, None)
it = self._iterators.pop(iid, None)
if hasattr(it, "close"):
it.close()

def server_status(self):
return self.server.server_status(self.storage_id)
Expand Down
2 changes: 2 additions & 0 deletions src/ZEO/asyncio/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,8 @@ def test_multiple_addresses(self):
transport = loop.transport
protocol.data_received(sized(self.enc + b'5'))
self.assertEqual(self.unsized(transport.pop(2)), self.enc + b'5')
# cancel the heartbeat to make debugging easier
protocol.heartbeat_handle.cancel()
self.respond(1, None)

# Now, when the first connection fails, it won't be retried,
Expand Down
1 change: 1 addition & 0 deletions src/ZEO/nagios.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ def check(addr, output_metrics, status, per):
try:
s.connect(addr)
except socket.error as err:
s.close()
return error("Can't connect %s" % err)

s.sendall(b'\x00\x00\x00\x04ruok')
Expand Down
10 changes: 9 additions & 1 deletion src/ZEO/tests/CommitLockTests.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,10 @@ def testrun(self):
self.myvote()
self.storage.tpc_finish(self.trans)
except ClientDisconnected:
pass
self.storage.tpc_abort(self.trans)
except Exception:
self.storage.tpc_abort(self.trans)
raise

def myvote(self):
# The vote() call is synchronous, which makes it difficult to
Expand Down Expand Up @@ -77,7 +80,11 @@ class CommitLockTests(object):
# This causes the commit lock code to be exercised. Once the
# other connections are started, the first transaction completes.

txn = None

def _cleanup(self):
if self.txn is not None:
self._storage.tpc_abort(self.txn)
for store, trans in self._storages:
store.tpc_abort(trans)
store.close()
Expand All @@ -88,6 +95,7 @@ def _start_txn(self):
self._storage.tpc_begin(txn)
oid = self._storage.new_oid()
self._storage.store(oid, ZERO, zodb_pickle(MinPO(1)), '', txn)
self.txn = txn
return oid, txn

def _begin_threads(self):
Expand Down
2 changes: 2 additions & 0 deletions src/ZEO/tests/ConnectionTests.py
Original file line number Diff line number Diff line change
Expand Up @@ -992,6 +992,7 @@ def checkTimeout(self):
else:
self.fail('bad logging')

storage.tpc_abort(txn)
storage.close()

def checkTimeoutOnAbort(self):
Expand Down Expand Up @@ -1038,6 +1039,7 @@ def checkTimeoutAfterVote(self):
# Load should fail since the object should not be in either the cache
# or the server.
self.assertRaises(KeyError, storage.load, oid, '')
storage.tpc_abort(t)


class MSTThread(threading.Thread):
Expand Down
2 changes: 2 additions & 0 deletions src/ZEO/tests/IterationTests.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ def checkIteratorGCStorageDisconnect(self):
# calling tpc_abort implicitly.
self._storage.notify_disconnected()
self.assertEqual(0, len(self._storage._iterator_ids))
# maybe, ``notify_disconnected`` should automatically clean up
self._storage.tpc_abort(t) # avoid ``ResourceWarning``

def checkIteratorParallel(self):
self._dostore()
Expand Down
5 changes: 4 additions & 1 deletion src/ZEO/tests/testZEO.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,12 @@ def test_ZEO_client_convenience(self):
import ZEO

client_thread = mock.Mock(
spec=['call', 'async', 'async_iter', 'wait'])
spec=['call', 'async', 'async_iter', 'wait', 'close'])
client = ZEO.client(
8001, wait=False, _client_factory=client_thread)
self.assertIsInstance(client, ClientStorage)
client.close()
client._cache.close() # client thread responsibility

def test_ZEO_DB_convenience_ok(self):
import mock
Expand Down Expand Up @@ -766,6 +768,7 @@ def checkTransactionBufferCleanup(self):
self._storage.tpc_begin(t)
self._storage.storeBlob(
oid, ZODB.utils.z64, 'foo', 'blob_file', '', t)
self._storage.tpc_abort(t)
self._storage.close()


Expand Down
4 changes: 4 additions & 0 deletions src/ZEO/tests/testZEO2.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ def proper_handling_of_blob_conflicts():
>>> logger.removeHandler(handler)
>>> zs2.tpc_abort('1')
>>> fs.close()
>>> server.close()
"""


Expand Down Expand Up @@ -192,6 +194,8 @@ def errors_in_vote_should_clear_lock():
True
>>> zs.tpc_abort('1')
>>> server.close()
"""


Expand Down
2 changes: 2 additions & 0 deletions src/ZEO/tests/test_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ def testEviction(self):

# TODO: Need to make sure eviction of non-current data
# are handled correctly.
cache.close()

def testSerialization(self):
self.cache.store(n1, n2, None, b"data for n1")
Expand Down Expand Up @@ -206,6 +207,7 @@ def testOldObjectLargerThanCache(self):
# If an object cannot be stored in the cache, it must not be
# recorded as non-current.
self.assertTrue(1 not in cache.noncurrent)
cache.close()

def testVeryLargeCaches(self):
cache = ZEO.cache.ClientCache('cache', size=(1 << 32)+(1 << 20))
Expand Down

0 comments on commit 9f586d5

Please sign in to comment.