diff --git a/extmod/vfs_fat_file.c b/extmod/vfs_fat_file.c index 13ad624ee22..bbd44db1d5b 100644 --- a/extmod/vfs_fat_file.c +++ b/extmod/vfs_fat_file.c @@ -182,7 +182,7 @@ static const mp_stream_p_t vfs_fat_fileio_stream_p = { MP_DEFINE_CONST_OBJ_TYPE( mp_type_vfs_fat_fileio, MP_QSTR_FileIO, - MP_TYPE_FLAG_ITER_IS_STREAM, + MP_TYPE_FLAG_ITER_IS_STREAM | MP_TYPE_FLAG_FINALISER_FIRST, print, file_obj_print, protocol, &vfs_fat_fileio_stream_p, locals_dict, &vfs_fat_rawfile_locals_dict @@ -198,7 +198,7 @@ static const mp_stream_p_t vfs_fat_textio_stream_p = { MP_DEFINE_CONST_OBJ_TYPE( mp_type_vfs_fat_textio, MP_QSTR_TextIOWrapper, - MP_TYPE_FLAG_ITER_IS_STREAM, + MP_TYPE_FLAG_ITER_IS_STREAM | MP_TYPE_FLAG_FINALISER_FIRST, print, file_obj_print, protocol, &vfs_fat_textio_stream_p, locals_dict, &vfs_fat_rawfile_locals_dict diff --git a/extmod/vfs_lfsx_file.c b/extmod/vfs_lfsx_file.c index c72fcf5b59d..f62aaa88bc8 100644 --- a/extmod/vfs_lfsx_file.c +++ b/extmod/vfs_lfsx_file.c @@ -223,7 +223,7 @@ static const mp_stream_p_t MP_VFS_LFSx(fileio_stream_p) = { MP_DEFINE_CONST_OBJ_TYPE( MP_TYPE_VFS_LFSx_(_fileio), MP_QSTR_FileIO, - MP_TYPE_FLAG_ITER_IS_STREAM, + MP_TYPE_FLAG_ITER_IS_STREAM | MP_TYPE_FLAG_FINALISER_FIRST, print, MP_VFS_LFSx(file_print), protocol, &MP_VFS_LFSx(fileio_stream_p), locals_dict, &MP_VFS_LFSx(file_locals_dict) @@ -239,7 +239,7 @@ static const mp_stream_p_t MP_VFS_LFSx(textio_stream_p) = { MP_DEFINE_CONST_OBJ_TYPE( MP_TYPE_VFS_LFSx_(_textio), MP_QSTR_TextIOWrapper, - MP_TYPE_FLAG_ITER_IS_STREAM, + MP_TYPE_FLAG_ITER_IS_STREAM | MP_TYPE_FLAG_FINALISER_FIRST, print, MP_VFS_LFSx(file_print), protocol, &MP_VFS_LFSx(textio_stream_p), locals_dict, &MP_VFS_LFSx(file_locals_dict) diff --git a/extmod/vfs_posix_file.c b/extmod/vfs_posix_file.c index 5fe330d7fcf..b1380003e2a 100644 --- a/extmod/vfs_posix_file.c +++ b/extmod/vfs_posix_file.c @@ -279,7 +279,7 @@ static const mp_stream_p_t vfs_posix_fileio_stream_p = { MP_DEFINE_CONST_OBJ_TYPE( mp_type_vfs_posix_fileio, MP_QSTR_FileIO, - MP_TYPE_FLAG_ITER_IS_STREAM, + MP_TYPE_FLAG_ITER_IS_STREAM | MP_TYPE_FLAG_FINALISER_FIRST, print, vfs_posix_file_print, protocol, &vfs_posix_fileio_stream_p, locals_dict, &vfs_posix_rawfile_locals_dict @@ -340,7 +340,7 @@ static void vfs_posix_textio_attr(mp_obj_t self_in, qstr attr, mp_obj_t *dest) { MP_DEFINE_CONST_OBJ_TYPE( mp_type_vfs_posix_textio, MP_QSTR_TextIOWrapper, - MP_TYPE_FLAG_ITER_IS_STREAM, + MP_TYPE_FLAG_ITER_IS_STREAM | MP_TYPE_FLAG_FINALISER_FIRST, print, vfs_posix_file_print, protocol, &vfs_posix_textio_stream_p, VFS_POSIX_TEXTIO_TYPE_ATTR diff --git a/main.c b/main.c index d7a0d31a941..d4d76be335d 100644 --- a/main.c +++ b/main.c @@ -438,6 +438,10 @@ static void cleanup_after_vm(mp_obj_t exception) { wifi_user_reset(); #endif + // Close files while their block devices and buses are still usable. + // Other finalisers run during stop_mp() after the ports have reset. + gc_run_finalisers_first(); + // reset_board_buses() first because it may release pins from the never_reset state, so that // reset_port() can reset them. #if CIRCUITPY_BOARD diff --git a/py/gc.c b/py/gc.c index 57bad018e76..fcb82ed9858 100644 --- a/py/gc.c +++ b/py/gc.c @@ -790,6 +790,58 @@ static void gc_deal_with_stack_overflow(void) { } } +#if MICROPY_ENABLE_FINALISER +static void gc_sweep_run_finaliser(const mp_state_mem_area_t *area, size_t block) { + if (ATB_GET_KIND(area, block) == AT_HEAD) { + mp_obj_base_t *obj = (mp_obj_base_t *)PTR_FROM_BLOCK(area, block); + if (obj->type != NULL) { + // if the object has a type then see if it has a __del__ method + mp_obj_t dest[2]; + mp_load_method_maybe(MP_OBJ_FROM_PTR(obj), MP_QSTR___del__, dest); + if (dest[0] != MP_OBJ_NULL) { + // load_method returned a method, execute it in a protected environment + #if MICROPY_ENABLE_SCHEDULER + mp_sched_lock(); + #endif + mp_call_function_1_protected(dest[0], dest[1]); + #if MICROPY_ENABLE_SCHEDULER + mp_sched_unlock(); + #endif + } + } + // clear finaliser flag + FTB_CLEAR(area, block); + } +} +#endif + +// Run finalisers for objects whose type requires an earlier shutdown pass. +// This pass is type-based because heap block order is not allocation order: +// GC can reuse free blocks below an existing object. +static void gc_sweep_run_finalisers_first(void) { + #if MICROPY_ENABLE_FINALISER + for (const mp_state_mem_area_t *area = &MP_STATE_MEM(area); area != NULL; area = NEXT_AREA(area)) { + assert(area->gc_last_used_block <= area->gc_alloc_table_byte_len * BLOCKS_PER_ATB); + size_t ftb_end = area->gc_last_used_block / BLOCKS_PER_FTB; + for (size_t ftb_idx = 0; ftb_idx <= ftb_end; ftb_idx++) { + byte ftb = area->gc_finaliser_table_start[ftb_idx]; + size_t block = ftb_idx * BLOCKS_PER_FTB; + while (ftb) { + MICROPY_GC_HOOK_LOOP(block); + if ((ftb & 1) && ATB_GET_KIND(area, block) == AT_HEAD) { + mp_obj_base_t *obj = (mp_obj_base_t *)PTR_FROM_BLOCK(area, block); + if (obj->type != NULL && (obj->type->flags & MP_TYPE_FLAG_FINALISER_FIRST)) { + gc_sweep_run_finaliser(area, block); + } + } + ftb >>= 1; + block++; + } + } + } + #endif +} + // Run finalisers for all to-be-freed blocks static void gc_sweep_run_finalisers(void) { #if MICROPY_ENABLE_FINALISER || MICROPY_PY_WEAKREF @@ -807,26 +859,7 @@ static void gc_sweep_run_finalisers(void) { while (ftb) { MICROPY_GC_HOOK_LOOP(block); if (ftb & 1) { // FTB_GET(area, block) shortcut - if (ATB_GET_KIND(area, block) == AT_HEAD) { - mp_obj_base_t *obj = (mp_obj_base_t *)PTR_FROM_BLOCK(area, block); - if (obj->type != NULL) { - // if the object has a type then see if it has a __del__ method - mp_obj_t dest[2]; - mp_load_method_maybe(MP_OBJ_FROM_PTR(obj), MP_QSTR___del__, dest); - if (dest[0] != MP_OBJ_NULL) { - // load_method returned a method, execute it in a protected environment - #if MICROPY_ENABLE_SCHEDULER - mp_sched_lock(); - #endif - mp_call_function_1_protected(dest[0], dest[1]); - #if MICROPY_ENABLE_SCHEDULER - mp_sched_unlock(); - #endif - } - } - // clear finaliser flag - FTB_CLEAR(area, block); - } + gc_sweep_run_finaliser(area, block); } ftb >>= 1; block++; @@ -834,18 +867,18 @@ static void gc_sweep_run_finalisers(void) { #endif #if MICROPY_PY_WEAKREF byte wtb = area->gc_weakref_table_start[ftb_idx]; - block = ftb_idx * BLOCKS_PER_WTB; + size_t weakref_block = ftb_idx * BLOCKS_PER_WTB; while (wtb) { - MICROPY_GC_HOOK_LOOP(block); + MICROPY_GC_HOOK_LOOP(weakref_block); if (wtb & 1) { // WTB_GET(area, block) shortcut - if (ATB_GET_KIND(area, block) == AT_HEAD) { - mp_obj_base_t *obj = (mp_obj_base_t *)PTR_FROM_BLOCK(area, block); + if (ATB_GET_KIND(area, weakref_block) == AT_HEAD) { + mp_obj_base_t *obj = (mp_obj_base_t *)PTR_FROM_BLOCK(area, weakref_block); gc_weakref_about_to_be_freed(obj); - WTB_CLEAR(area, block); + WTB_CLEAR(area, weakref_block); } } wtb >>= 1; - block++; + weakref_block++; } #endif } @@ -853,6 +886,13 @@ static void gc_sweep_run_finalisers(void) { #endif // MICROPY_ENABLE_FINALISER || MICROPY_PY_WEAKREF } +void gc_run_finalisers_first(void) { + gc_collect_start_common(); + gc_sweep_run_finalisers_first(); + MP_STATE_THREAD(gc_lock_depth) &= ~GC_COLLECT_FLAG; + GC_EXIT(); +} + // Free unmarked heads and their tails static void gc_sweep_free_blocks(void) { #if MICROPY_PY_GC_COLLECT_RETVAL diff --git a/py/gc.h b/py/gc.h index 0f693d49222..c55d43d8c94 100644 --- a/py/gc.h +++ b/py/gc.h @@ -38,6 +38,8 @@ void gc_init(void *start, void *end); // CIRCUITPY-CHANGE void gc_deinit(void); +// Run finalisers marked to run before the normal shutdown sweep. +void gc_run_finalisers_first(void); #if MICROPY_GC_SPLIT_HEAP // Used to add additional memory areas to the heap. diff --git a/py/obj.h b/py/obj.h index 368ac96dbd2..bb93c635d4e 100644 --- a/py/obj.h +++ b/py/obj.h @@ -611,6 +611,8 @@ typedef mp_obj_t (*mp_fun_kw_t)(size_t n, const mp_obj_t *, mp_map_t *); #define MP_TYPE_FLAG_SUBSCR_ALLOWS_STACK_SLICE (0x0400) // CIRCUITPY-CHANGE: check for valid types in json dumps #define MP_TYPE_FLAG_PRINT_JSON (0x0800) +// Run this type's finalisers before other finalisers when the VM is shutting down. +#define MP_TYPE_FLAG_FINALISER_FIRST (0x1000) typedef enum { PRINT_STR = 0, diff --git a/py/objpolyiter.c b/py/objpolyiter.c index 65c1f182ec8..f3e8b94fc10 100644 --- a/py/objpolyiter.c +++ b/py/objpolyiter.c @@ -79,7 +79,7 @@ static MP_DEFINE_CONST_DICT(mp_obj_polymorph_iter_locals_dict, mp_obj_polymorph_ MP_DEFINE_CONST_OBJ_TYPE( mp_type_polymorph_iter_with_finaliser, MP_QSTR_iterator, - MP_TYPE_FLAG_ITER_IS_ITERNEXT, + MP_TYPE_FLAG_ITER_IS_ITERNEXT | MP_TYPE_FLAG_FINALISER_FIRST, iter, polymorph_it_iternext, locals_dict, &mp_obj_polymorph_iter_locals_dict ); diff --git a/tests/circuitpython-manual/storage/sdcard_open_file_soft_reload.py b/tests/circuitpython-manual/storage/sdcard_open_file_soft_reload.py new file mode 100644 index 00000000000..b10b6ff1d83 --- /dev/null +++ b/tests/circuitpython-manual/storage/sdcard_open_file_soft_reload.py @@ -0,0 +1,44 @@ +# Manual regression test for closing an open SD-card file on soft reload. +# Run on a board with board.SD_CS and an SD card. Delete +# /sdcard-reload-test-state.txt from CIRCUITPY and +# /sdcard-reload-test.txt from the SD card before starting. + +import board +import gc +import sdcardio +import storage +import supervisor + +STATE_FILE = "/sdcard-reload-test-state.txt" +SD_FILE = "/sd/sdcard-reload-test.txt" +EXPECTED = "unflushed write survives soft reload" + +# Leave free blocks below SDCard for later allocations to reuse. +heap_space = bytearray(4096) +sd = sdcardio.SDCard(board.SPI(), board.SD_CS) +del heap_space +gc.collect() +storage.mount(storage.VfsFat(sd), "/sd") + +try: + with open(STATE_FILE, "r") as state_file: + state = state_file.read() +except OSError: + state = "" + +if state != "pending": + with open(STATE_FILE, "w") as state_file: + state_file.write("pending") + + # Deliberately keep this file open and unflushed as the VM is stopped. + test_file = open(SD_FILE, "w") + test_file.write(EXPECTED) + print("Wrote test data; reloading without closing the SD file.") + supervisor.reload() +else: + with open(SD_FILE, "r") as test_file: + actual = test_file.read() + if actual == EXPECTED: + print("PASS: open SD-card file was closed before its SPI bus reset.") + else: + print("FAIL: expected %r, got %r" % (EXPECTED, actual))