[RP2] machine.soft_reset() hangs when i2s is not deinitialized

[marecl]

Checks

• I agree to follow the MicroPython Code of Conduct to ensure a safe and respectful space for everyone.

• I've searched for existing issues matching this bug, and didn't find any.

Port, board and/or hardware

RPI_PICO

MicroPython version

MicroPython v1.22.2 on 2024-02-22; Raspberry Pi Pico with RP2040

Reproduction

1. Initialize I2S device:

from machine import I2S
from machine import Pin

audio_out = I2S(0,
	sck=Pin(0), ws=Pin(1), sd=(2),
	mode=I2S.TX,
	bits=16,
	format=I2S.MONO,
	rate=44100,
	ibuf=20000)

2. Call machine.soft_reset()
3. Repeat 1, call audio_out.deinit() and no. 2 again

Expected behaviour

machine.soft_reset() resets machine

Observed behaviour

device hangs, requiring hardware reset or reconnecting USB

Additional Information

Verified on latest git commit and official release 1.22.2. This issue is pin-independent.

[marecl]

Solved in mentioned PR. Waiting for feedback on the issue.

[marecl]

#14345

[dpgeorge]

Thanks for the detailed report and reproduction. I can confirm the issue on a Pico. And the same issue exists on the stm32 port (tested with a PYBv1.0), and I assume it also exists on the mimxrt port.

The fix would be to enable finalisers on I2S objects for all the ports, following how esp32 does it. That would also fix other cases of I2S instances being GC'd without .deinit() being called on them by the user.

[robert-hh]

The mimxrt port de-inits I2S on soft reset. But as for enabling finalizers: during soft reset gc_sweep_all() is called. I had the impression that this call clears each and every object from the heap, whether created with finalizer or not.

[robert-hh]

The i2s object contains usually pointers to buffers, allocated with m_new(). Assuming that gc_sweep_all() does not free objects in a specific order it may happen, machine_is2_deinit() as part of calling del with it is called after the buffers have already been cleared.
What happens is m_free() is called on already free'd object?

[dpgeorge]

The mimxrt port de-inits I2S on soft reset

Ah, so it does! It could instead use finalisers, that would have the same effect.

during soft reset gc_sweep_all() is called. I had the impression that this call clears each and every object from the heap, whether created with finalizer or not.

Yes, gc_sweep_all() clears all sections of the GC heap that have been allocated, marking them as free. As part of this it runs finalisers on those bits of memory that are freed and that have a finaliser associated with them. So you must allocate the memory with mp_obj_malloc_with_finaliser() and have a __del__ method on that type (otherwise how does the GC know what to call to finalise it?).

Assuming that gc_sweep_all() does not free objects in a specific order it may happen, machine_is2_deinit() as part of calling del with it is called after the buffers have already been cleared.
What happens is m_free() is called on already free'd object?

That's a very good point. If you call m_free() on an object that's already been free'd then an assertion will fail (if assertions are enabled).

So this needs some more thought... maybe we need a gc_run_all_finalisers() which executes all finalisers first, and then run gc_sweep_all().