weakref: Don't leak cyclic weakrefs.

[AJMansfield]

Summary

In micropython#18822 (review), I discussed how that PRs implementation of weakref leaks weakrefs whose callbacks are closures of their target, with a discussion of the underlying mathematics and one primitive way to resolve this issue.

This PR proposes a more refined version of that resolution, achieved by moving mp_weakref_map table from the root pointer section of mp_state_ctx.vm to a non-root-pointer part of mp_state_ctx.mem and adding it to the GC's root closure as a separate step, so that membership in that table is only considered for the gc_sweep_free_blocks step, and not for the gc_sweep_run_finalisers step.

Testing

This PR adds an automated test that verifies the lack of a leak (with very loose bounds). I've checked this test for sensitivity against Unix, and can confirm that it passes on Unix and RP2.

I've rigorously checked the underlying mathematical theory. While it's not impossible that I've made an error in my proofs I'm confident that this change is theoretically sound --- this change maintains the GC's "conservativeness" guarantee and still prevents python code from forming an invalid.

Trade-offs and Alternatives

Rerunning a root-collection pass from this code region does come with a minor increase to code size and runtime; however, since this is able to re-use functionality that already existed in the garbage collector this extra increase should be minimal.

[dpgeorge]

Thanks for this, it's interesting.

But consider the following case: a weakref.finalize() holds on to (as args) an unrelated object that has a __del__ itself, eg a file. If that file is only reachable from the weakref.finalize() then it'll have its finaliser run but then it won't be reclaimed (because it'll be reached when scanning the weakref map). Each GC cycle it'll run its finaliser again but not be reclaimed (until the weakref.finalize() is resolved itself).

Eg:

l = []
def callback(arg):
    pass
file = <some opened file>
weakref.finalize(l, callback, file)
gc.collect()  # will close the file!
gc.collect()  # will close the file again!
l = None
gc.collect()  # file is closed again, callback runs, everything is reclaimed

Am I correct about what will happen there?

[AJMansfield]

The FTB_CLEAR(area, block) operation at the end of the finaliser call prevents any object's finaliser from being run a second time; once an object's finaliser runs it's then permanently a post-final object on any subsequent GC pass even if it somehow were reattached to the object graph in the mean time.

Though, you're right that there's still the possibility of this being unexpected if the finaliser's file argument ends up closed even once before the finaliser runs.

[dpgeorge]

The FTB_CLEAR(area, block) operation at the end of the finaliser call prevents any object's finaliser from being run a second time

Ah, yes, you're right. It'll only run once.

But still, it's left in a kind of half-finalised state. And application code would definitely not expect a file to be closed before it's really ready to be GC'd. Eg the callback in my example above would expect the file to be still open so it can write to it.

[AJMansfield]

Took me a moment of puzzling through how or even whether micropython#18840 avoids that, too --- the reference inserted into the target's __weakref__ ends up keeping the file open and ensures that the finalize will run in the same GC generation as file.__del__, but there's nothing that enforces any kind of sequencing there.

TBH it's probably worth testing the comparison to CPython here too --- this feels like a limitation that could easily be present there too.

[dpgeorge]

Took me a moment of puzzling through how or even whether micropython#18840 avoids that, too --- the reference inserted into the target's __weakref__ ends up keeping the file open and ensures that the finalize will run in the same GC generation as file.__del__, but there's nothing that enforces any kind of sequencing there.

Yes, that's a good point, it looks like micropython#18840 can finalise args/kwargs before running finalize's __del__ method. You probably need to explicitly retain a pointer to all alive finalize objects so they stay reachable until the thing they weakref is gone.

TBH it's probably worth testing the comparison to CPython here too --- this feels like a limitation that could easily be present there too.

Looking at CPython's implementation, it keeps strong references to all finalize objects (and their args/kwargs) in a big dict. So it won't prematurely finalise any of the args/kwargs.