Core: fix some memory leak sources without removing caching

[black-sliver]

Supersedes #2399

Use of @Utils.cache_self1 gives a "Parameter(s) unfilled" on call site in pycharm, but mypy claims it's fine. If anyone knows if we can fix that, please let me know.

Maybe @el-u with the typing experience.

[Berserker66]

This is in a multiworld with a yaml of each game, except sudoku and ff1

[Berserker66]

With just Subnautica it now deallocates the multiworld as desired. Remaining culripits are probably in the world folders. Don't know if that should be adressed in this PR or a follow-up, but as is, this PR is an improvement.

[black-sliver]

I'd like to see if someone can figure out the pycharm linter error.

Two things to check in the meantime are
if cached_property is fine or not, and
if the culprit is Zillion, since that has an internal cache.

If you beat me to it, feel free to push to my branch.

[Berserker66]

cached_property was hit during the Subnautica run which did de-allocate. To make sure I just did:

And that too de-allocates.

[Berserker66]

genning with just zillion also de-allocates.

[el-u]

I'm not seeing "Parameter(s) unfilled" on the new code in BaseClasses.

[black-sliver]

It highlights the closing ) on line 1235, for example, and when I hover it, i get this

[Berserker66]

I think that's what's solved with typing.Self on newer python.

[el-u]

It highlights the closing ) on line 1235, for example, and when I hover it, i get this

I see. Honestly, this could be a PyCharm issue. The typing seems correct.

[black-sliver]

I think that's what's solved with typing.Self on newer python.

I don't think so, because PyCharm claims the number of arguments is wrong, not the type.

Honestly, this could be a PyCharm issue.

There are a couple of issues open for PyCharm with decorator and typing, so it's very likely. Typing also looks correct to mypy and me.

[Berserker66]

Find which issue it is and leave a comment... on both ends, I'd say.

[el-u]

Can you make a test that creates a multiworld for each world type and checks whether it can be deallocated?

[black-sliver]

Can you make a test that creates a multiworld for each world type and checks whether it can be deallocated?

• We could check for memory leak as part of test.bases.WorldTestBase.tearDown, however that should probably be limited to once per class, because gc.collect() is expensive - i.e. tearDownClass should run for the last one, I think. We can also only warn there I think, but it would then test all option combinations for worlds that define tests.

• We could create a generic test (like the ones test_reachability.py) that asserts memory is free'd, however that would only run for default options.

• We could do both, and skip the tearDown one for options == {}

[black-sliver]

Maybe like this @el-u ?

[Berserker66]

Are we sure the current test failures now are accurate? It not leaking on 3.11 feels odd.

[el-u]

Are we sure the current test failures now are accurate? It not leaking on 3.11 feels odd.

No. I'd say the test failures are not understood. It fails for worlds that deallocate properly when just running generate. Could be that there's something wrong with the test. Failing worlds are basically LADX + Minecraft + those that use the new options system; but unsure whether that has anything to do with it.

[black-sliver]

Those failure are bogus and only happen on py<3.11. I've been trying to figure out why, but I hit a dead end again. The references to the MultiWorld are all regions, but I am not sure where they are referenced - I'm afraid that might be somewhere inside unittest? An assertion or a subtest maybe?

I think I'll just disable the failing ones for <3.11 for now and leave a comment. The generic (default options) test still works for those at least.