Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix HomeServers leaking during trial test runs #15630

Merged
merged 3 commits into from
May 19, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix HomeServers leaking due to #15334
When the gen-0 GC is run at the end of each test, the `TestCase` object
usually still holds references to the `HomeServer` used during the test.
As a result, the `HomeServer` gets promoted to gen-1 and then never
garbage collected.

Fix this by periodically running full GCs.

Signed-off-by: Sean Quah <seanq@matrix.org>
  • Loading branch information
Sean Quah committed May 18, 2023
commit cc480e10f4ff31cde7911df822c6c7758db34c6b
11 changes: 9 additions & 2 deletions tests/unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,13 +229,20 @@ def tearDown(orig: Callable[[], R]) -> R:
#
# The easiest way to do this would be to do a full GC after each test
# run, but that is very expensive. Instead, we disable GC (above) for
# the duration of the test so that we only need to run a gen-0 GC, which
# is a lot quicker.
# the duration of the test and only run a gen-0 GC, which is a lot
# quicker. This doesn't clean up everything, since the TestCase
# instance still holds references to objects created during the test,
# such as HomeServers, so we do a full GC every so often.

@around(self)
def tearDown(orig: Callable[[], R]) -> R:
ret = orig()
gc.collect(0)
# Run a full GC every 50 gen-0 GCs.
gen0_stats = gc.get_stats()[0]
gen0_collections = gen0_stats["collections"]
if gen0_collections % 50 == 0:
gc.collect()
Comment on lines +241 to +245
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea why we're manually kicking the GC at all here? Is this to try and make tests more deterministic somehow? (Are we relying on __del__ being called somewhere?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failed Deferreds that haven't been awaited on / consumed will error out when they are GCed (and fail the currently running test somehow). If we let the garbage collector run at its own leisure, such failures would be logged against a random future test. At least I think that's what the original code is trying to trigger.

gc.enable()
set_current_context(SENTINEL_CONTEXT)

Expand Down