static analysis finds problem in gc_never_free

[jepler]

The full report from scan-build-7 is here: http://media.unpythonic.net/emergent-files/sandbox/report-3d3d49.html#LN954

Basically, in the case where the first block of 3 "never free" objects is filled, the code needs to allocate a new block and hook it into the linked list that goes via the [0] index of each block. The logic to chain the blocks together erroneously tries to use current_reference_block[0] but current_reference_block is guaranteed to be NULL at this point, by the condition of the while loop.

This bug isn't triggering in practice at this time, as gc_never_free sees only limited use from displayio and I think this means at most one entry is ever used.

[furbrain]

I've managed to trigger this bug with this code

import displayio
import adafruit_displayio_sh1106
import time
import busio
import board

def setup():
    print("initialising i2c")
    i2c = busio.I2C(board.D8, board.D7)
    print("initialising bus")
    bus = displayio.I2CDisplay(i2c, device_address=0x3c)
    print("initialising display")
    disp = adafruit_displayio_sh1106.SH1106(bus, width=132,height=64)
    time.sleep(0.1)
    displayio.release_displays()
    i2c.deinit()
    time.sleep(0.1)
    
for i in range(10):
    print(f"loop {i}")
    time.sleep(0.5)
    setup()

This gives the following output:

[tio 21:22:04] Connected
loop 0
free: 140176
initialising i2c
initialising bus
initialising display
loop 1
free: 140144
initialising i2c
initialising bus
initialising display
loop 2
free: 140128
initialising i2c
initialising bus
initialising display
loop 3
free: 140112
initialising i2c
initialising bus

[tio 21:22:12] Disconnected
[tio 21:22:14] Connected
Auto-reload is off.
Running in safe mode! Not running saved code.

You are in safe mode because:
CircuitPython core code crashed hard. Whoops!
Fault detected by hardware.
Please file an issue with your program at https://github.com/adafruit/circuitpython/issues.
Press reset to exit safe mode.

Press any key to enter the REPL. Use CTRL-D to reload.

As you can see this fits with the above issue - we successfully create the first block of pointers, and can allocate three items to the never_free list, but it then crashes when we try to create a second block of pointers

[furbrain]

I've submitted a fix for this, and now my code works as expected:

code.py output:
loop 0
free: 140176
initialising i2c
initialising bus
initialising display
loop 1
free: 140144
initialising i2c
initialising bus
initialising display
loop 2
free: 140128
initialising i2c
initialising bus
initialising display
loop 3
free: 140112
initialising i2c
initialising bus
initialising display
loop 4
free: 140080
initialising i2c
initialising bus
initialising display
loop 5
free: 140064
initialising i2c
initialising bus
initialising display
loop 6
free: 140048
initialising i2c
initialising bus
initialising display
loop 7
free: 139984
initialising i2c
initialising bus
initialising display
loop 8
free: 139968
initialising i2c
initialising bus
initialising display
loop 9
free: 139952
initialising i2c
initialising bus
initialising display

Code done running.

Press any key to enter the REPL. Use CTRL-D to reload.

There is however another issue: there is now a memory leak as we add a new i2c bus reference for each time around my loop

My application will maybe do 50 loops before getting a hard reset, so I can live with this for now. Fixing would likely require a new function like gc_allow_free(ptr) to remove the pointer from the list - this could be called by displayio.release_displays at the relevant point

[furbrain]

I can work on creating gc_allow_free if there is an appetite for it

[tannewt]

I can work on creating gc_allow_free if there is an appetite for it

I'm happy to review if you think it'll help these situations.

[tannewt]

I can work on creating gc_allow_free if there is an appetite for it

After reviewing the PR, I think it'd be better to remove gc_never_free() and have the display code manage collecting those pointers through displayio_gc_collect().

[jepler]

thanks @furbrain! did you encounter this problem "organically" or did you go looking for it?

[dhalbert]

Fixed by #7983.

[furbrain]

@jepler I encountered it organically - I have a display that i need to regularly power down and then reinitialise and was encountering this bug.

@tannewt I agree - I think you said that only one display is actually used by CP internals for displaying error messages etc - so may be better just to keep a reference to that one display.