Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.43%. Comparing base (5114f2c) to head (d7ba712).

❗ Current head d7ba712 differs from pull request most recent head d0528fe. Consider uploading reports for the commit d0528fe to get more accurate results

@@            Coverage Diff             @@
##           master   #13011      +/-   ##
==========================================
+ Coverage   98.39%   98.43%   +0.03%     
==========================================
  Files         161      158       -3     
  Lines       21200    20978     -222     
==========================================
- Hits        20860    20649     -211     
+ Misses        340      329      -11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This is an automated heads-up that we've just merged a Pull Request
that removes the STATIC macro from MicroPython's C API.

See #13763

A search suggests this PR might apply the STATIC macro to some C code. If it
does, then next time you rebase the PR (or merge from master) then you should
please replace all the STATIC keywords with static.

Although this is an automated message, feel free to @-reply to me directly if
you have any questions about this.

This PR has been rebased and the comments addressed. Ready for re-review thanks!

@dpgeorge there's something strange going on with running the finalizer, or not.

This PR adds the print statement in __del__ which is reflected in the unit test exp.

This test is failing however, never seeing that new printout.

I've added two new smaller tests to explicitly trigger the collect, which are both passing here in CI now. However in my home computer one of them fails.

This works:

timer = cexample.Timer()
timer = None
gc.collect()
print("done")

This fails to run the finalizer

a = 100
b = 20
timer = cexample.Timer()
c = 4
d = 5
timer = None
gc.collect()
print("done")

Adding a time.sleep_ms(100) instead of c or d lines stops it working too, though replacing both makes it work again.

Could try to gc.collect a couple of times, that usually does the trick

Could try to gc.collect a couple of times, that usually does the trick

Yeah I did try that earlier, tried to trim the repros down to the minimum. Just running it twice didn't seem to fix it. I also had cases like:

a = 100
b = 20
timer = cexample.Timer()
c = 4
d = 5
timer = None
timer = cexample.Timer()
timer = cexample.Timer()
timer = None

gc.collect()
print("done")

where the collect would run the finalizer in the final two instances (verified with extra print(self) in C code) but the one for the original instance never gets run.

It can be finicky.. E.g. I have this somewhere to force TheThing to be collected:

a = 1
b = 2
c = 3

def allocate_some():
  d = [a] * 100
  e = [3] * 100
  f = [4] * 100

def fun():
  x = TheThing()

fun()
allocate_some()

gc.collect()

silly thing is that back when I wrote it I spent some time figuring out on the gc level why exactly that extra code was needed but didn't write it down :(

That's really interesting, I got the impression that the number of allocations before and after was key here.

More importantly, I need to test whether it's always reliability run on soft-reboot; the point of this example / usage pattern is to use a finalizer to deinit hardware resources or similar, if that's not run when needed then it's nota usable pattern.

I got the impression that the number of allocations before and after was key here.

That certainly is possible, I don't think the allocation size really matters, rather 'poking' gc and/or stack; note my particular example here was written a long time ago (about 10 years I think) so things might have been a bit different then, and I haven't tried to minimize it, on the contrary: it started off a lot simpler but then I got failures on unix/windows ports depending on where they'd run (locally vs Travis vs newer compiler versions etc) and just added more statements to make the failures go away..

Yeah that sounds like what I was doing with these examples, adding more/less to them until the worked/didn't work.

As a more important test I just added the example C module to a pyboard-d build and ran it;

anl@STEP: ~/micropython $ mpremote connect /dev/ttyACM2
Connected to MicroPython at /dev/ttyACM2
Use Ctrl-] or Ctrl-x to exit this shell
>>> import cexample
>>> timer = cexample.Timer()
>>>
MPY: sync filesystems
MPY: soft reboot
de-init cexample resources
MicroPython v1.23.0-preview.324.gd0528fe571.dirty on 2024-04-08;
PYBD-SF6W with STM32F767IIK
Type "help()" for more information.

The deinit string is shown on soft reset - that's the actual requirement for this kind of automatic deinit and it worked. So this example is still relevant and ready for review. I'll need to figure out how I want to clean up the unit test though.

Trying to prompt gc to collect it at runtime should be less important; if user runtime deinit is needed a separate function should be exposed to do so.

rather 'poking' gc and/or stack

FWIW I think this will usually come down to stray pointers being left on the memory used for the stack, if a particular stack frame doesn't rewrite every word. It's hard to avoid this, and calling extra functions or evaluating additional Python code should cause them to be rewritten eventually.

There was a bug where this happened very reliably for some addresses - #14136 - but looks like the fix was included in the most recent rebase so probably this is unrelated if you're still seeing some "zombie" objects.

This PR still needs some work to get the tests running under CI. But I'm not sure what the best way is to do that. Maybe put the creation of the Timer object in a function and then GC after the function returns?

Also, making the Timer have a finaliser complicates the very simple Timer example class. I would suggest putting the finaliser on AdvancedTimer, but then you'd need to copy the make_new and locals_dict...

Preferred approach now in #16063