Close open files before resetting buses

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

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)

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

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

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,52 +859,40 @@ 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++;
             }
             #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
         }
     }
     #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

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.

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,

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
     );

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))