Skip to content

Commit

Permalink
*: Break many cycles
Browse files Browse the repository at this point in the history
The commit target and cyclic garbage created during tests. For the analysis, a `zope.testrunner` enhancement ("zopefoundation/zope.testrunner#133") has been important.

...

I believe that the ZEO implementation no longer creates cyclic garbage of its own. However, most tests still produce cyclic garbage. The analysis found 3 causes:
1. Python 3 (at least) creates a reference cycle for each class definition (between the class and its `mro'). This means that (most) local class definitions result in cyclic garbage. `unittest.mock` (prominently used by some tests) makes extensive use of local class definitions.
2. `asyncio` (at least in Python 3.9) can create cyclic garbage
3. `zope.interface` can create cyclic garbage in its implementation of `alsoProvides`

The cyclic garbage collector can free the cyclic structures; thus, there is no serious problem.

Extracted from #195
More details at: #195 (comment)
  • Loading branch information
d-maurer authored and navytux committed Dec 30, 2022
1 parent 9f586d5 commit 0dc7af9
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 6 deletions.
2 changes: 2 additions & 0 deletions src/ZEO/ClientStorage.py
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,8 @@ def close(self):
if self._check_blob_size_thread is not None:
self._check_blob_size_thread.join()

self.ping = self.sync = None # break reference cycles

_check_blob_size_thread = None

def _check_blob_size(self, bytes=None):
Expand Down
2 changes: 2 additions & 0 deletions src/ZEO/StorageServer.py
Original file line number Diff line number Diff line change
Expand Up @@ -939,6 +939,8 @@ def close(self, join_timeout=1):
if self.__thread is not None:
self.__thread.join(join_timeout)

self.acceptor = self.loop = None # break reference cycles

def close_conn(self, zeo_storage):
"""Remove the given zeo_storage from self.zeo_storages_by_storage_id.
Expand Down
1 change: 1 addition & 0 deletions src/ZEO/asyncio/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ def connection_made(self, transport):
sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, True)

# set up lower level sized message protocol
# creates reference cycle
smp = self.sm_protocol = SizedMessageProtocol(self._first_message)
smp.connection_made(transport) # takes over ``transport``
self.data_received = smp.data_received
Expand Down
21 changes: 17 additions & 4 deletions src/ZEO/asyncio/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ def close(self, exc=None):
logger.debug('closing %s: %s', self, exc)
connecting = not self._connecting.done()
cancel_task(self._connecting)
self._connecting = None # break reference cycle
for future in self.pop_futures():
if not future.done():
future.set_exception(ClientDisconnected(exc or "Closed"))
Expand All @@ -155,7 +156,7 @@ def close(self, exc=None):
if uvloop_used and connecting:
if not closing.done():
closing.set_result(True)

self.client = None # break reference cycle

def pop_futures(self):
# Remove and return futures from self.futures. The caller
Expand Down Expand Up @@ -211,8 +212,10 @@ def connection_lost(self, exc):
if not f.done():
f.set_exception(ClientDisconnected(exc or "Closed"))
else:
client = self.client
# will set ``self.client = None``
self.close(exc or "Connection lost")
self.client.disconnected(self)
client.disconnected(self)

@future_generator
def finish_connection(self, protocol_version):
Expand Down Expand Up @@ -839,6 +842,10 @@ def _(f):
future.set_exception(f.exception())
else:
future.set_result(None)
# break reference cycles
for name in Protocol.client_delegated:
delattr(self, name)
self.client = self.cache = None

# server callbacks
def invalidateTransaction(self, tid, oids):
Expand Down Expand Up @@ -915,8 +922,10 @@ def io_call(meth, *args, **kw):
return self.wait_for_result(result, self.timeout)
else:
raise
finally:
del result # break reference cycle in case of exception

self.io_call = io_call
self.io_call = io_call # creates reference cycle

def wait_for_result(self, future, timeout):
try:
Expand Down Expand Up @@ -1003,6 +1012,7 @@ def close(self):
f = base.create_future(loop)
self.client.close_threadsafe(f, False)
loop.run_until_complete(f)
self.__args = None # break reference cycle

@staticmethod
def _io_call_after_closure(*args, **kw):
Expand Down Expand Up @@ -1106,8 +1116,11 @@ def close(self):
# ``loop`` will be closed in the IO thread
# after stop processing
self.thread.join(9) # wait for the IO thread to terminate
if self.exception:
if self.exception:
try:
raise self.exception
finally:
self.exception = None # break reference cycle

def is_closed(self):
return self.__closed
Expand Down
3 changes: 3 additions & 0 deletions src/ZEO/asyncio/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ def close(self):
logger.debug("Closing server protocol")
if not self.closed:
super(ServerProtocol, self).close() # will set ``closed``
self.zeo_storage = None # break reference cycle

connected = None # for tests

Expand Down Expand Up @@ -301,3 +302,5 @@ def timeout():

# But if the server doesn't close in a second, stop the loop anyway.
loop.call_later(1, timeout)

self.server = None # break reference cycle
3 changes: 3 additions & 0 deletions src/ZEO/asyncio/smp.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ def data_received(data):
logger.exception("Processing message `%r` failed"
% collected)

# the following introduces a reference cycle broken in ``close``
self.data_received = data_received

def set_receive(self, receive):
Expand All @@ -167,6 +168,8 @@ def close(self):
return
self.__closed = True
self.transport.close()
# break reference cycles
self.transport = self.receive = self.data_received = None

# We define ``connection_lost`` to close the transport
# in order to avoid a ``ResourceWarning``
Expand Down
6 changes: 4 additions & 2 deletions src/ZEO/asyncio/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@ class Loop(object):
it does not check calls to non threadsafe methods.
"""

protocol = transport = None

def __init__(self, addrs=(), debug=True):
self.protocol = self.transport = None
self.addrs = addrs
self.get_debug = lambda: debug
self.connecting = {}
Expand Down Expand Up @@ -86,6 +85,8 @@ def call_exception_handler(self, context):
closed = False

def close(self):
# break reference cycles
Loop.__init__(self)
self.closed = True

def create_future(self):
Expand Down Expand Up @@ -252,6 +253,7 @@ def close(self):
# be exactly one ``connection_lost`` call.
if not self.protocol.connection_lost_called:
self.protocol.connection_lost(None)
self.protocol = None # break reference cycle

def get_extra_info(self, name):
return self.extra[name]
Expand Down
14 changes: 14 additions & 0 deletions src/ZEO/asyncio/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ def tearDown(self):
loop = self.loop
if loop is not None:
self.assertEqual(loop.exceptions, [])
# ``mock`` creates cyclic structures; break the cycles
_break_mock_cycles(self.target)

# For normal operation all (server) interface calls are synchronous:
# they wait for the result.
Expand Down Expand Up @@ -1228,6 +1230,18 @@ def check(*split_size):
check(4, len(msg), 4, len(msg))


def _break_mock_cycles(m):
"""break (``mock`` introduced) cycles in mock *m*.
Do not break cycles due to mock calling.
"""
m._mock_parent = m._mock_new_parent = None
for c in m._mock_children.values():
_break_mock_cycles(c)
if isinstance(m._mock_return_value, mock.Mock):
_break_mock_cycles(m._mock_return_value)


def test_suite():
suite = unittest.TestSuite()
suite.addTest(unittest.makeSuite(ClientTests))
Expand Down
1 change: 1 addition & 0 deletions src/ZEO/tests/servertesting.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ def close(*args, **kw):
pr_close()
zs.notify_disconnected()
protocol.loop.close()
del protocol.close # break reference cycle

protocol.close = close # install proper closing
zs.notify_connected(protocol)
Expand Down

0 comments on commit 0dc7af9

Please sign in to comment.