pycom / pycom-micropython-sigfox

A fork of MicroPython with the ESP32 port customized to run on Pycom's IoT multi-network modules.
MIT License
196 stars 167 forks source link

LFS: Closing a file twice crashes the firmware #523

Closed robert-hh closed 2 years ago

robert-hh commented 3 years ago

Version 1.20.2.r4, although I guess that this does not matter The three code lines below cause a crash by abort() being called by RTOS. The same code snippet behaves well with FAT, behaves well with LFS on micropython.org firmware. A dir(f) call after the first f.close() causes the firmware to freeze. On mp.org firmware, it still shows the file methods. That indicates that the file object gets corrupted on close. Until now, I did not find the place where that happens. Even if closing a file twice is not reasonable, the firmware should be able to deal with it, either by ignoring or raising an error. Code lines:

f = open("main.py")
f.close()
f.close()

start of decoded trace dump:

BT-0: 0x40090638 is in invoke_abort (/home/robert/Downloads/MicroPython/pycom-esp-idf/components/esp32/panic.c:156).
156         *((int *) 0) = 0;
BT-1: 0x40090885 is in abort (/home/robert/Downloads/MicroPython/pycom-esp-idf/components/esp32/panic.c:171).
171     invoke_abort();
BT-2: 0x40098170 is in multi_heap_free_impl (/home/peter/docs/pycom-esp-idf/components/heap/multi_heap_platform.h:54).
54          abort();
BT-3: 0x400852b2 is in heap_caps_free (/home/peter/docs/pycom-esp-idf/components/heap/heap_caps.c:268).
268     multi_heap_free(heap->heap, ptr);
BT-4: 0x40084dc5 is in _free_r (/home/peter/docs/pycom-esp-idf/components/newlib/syscalls.c:42).
42      heap_caps_free( ptr );
BT-6: 0x400f7e2a is in lfs_file_rawclose (littlefs/lfs_util.h:226).
226     free(p);
BT-7: 0x400f89c5 is in lfs_file_close (littlefs/lfs.c:5089).
5089        err = lfs_file_rawclose(lfs, file);
BT-8: 0x400f9676 is in littlefs_close_common_helper (littlefs/vfs_littlefs.c:326).
326     int res = lfs_file_close(lfs, fp);
BT-9: 0x400f973f is in file_obj_ioctl (littlefs/vfs_littlefs_file.c:105).
105             int res = littlefs_close_common_helper(&self->littlefs->lfs, &self->fp, &self->cfg, &self->timestamp_update);
BT-10: 0x4010c97d is in mp_stream_close (../py/stream.c:422).
422     mp_uint_t res = stream_p->ioctl(stream, MP_STREAM_CLOSE, 0, &error);
robert-hh commented 3 years ago

Carrying over the way mp.org is doing that, changing the section for close in vfs_littlefs_file.x, line 102cc prohibits closing a closed file. Next candidate for a PR.

    } else if (request == MP_STREAM_CLOSE) {
        if (self->littlefs == NULL) {
            return 0;
        }

        xSemaphoreTake(self->littlefs->mutex, portMAX_DELAY);
            int res = littlefs_close_common_helper(&self->littlefs->lfs, &self->fp, &self->cfg, &self->timestamp_update);
        xSemaphoreGive(self->littlefs->mutex);

        self->littlefs = NULL; // indicate a closed file
        if (res < 0) {
            *errcode = littleFsErrorToErrno(res);
            return MP_STREAM_ERROR;
        }
        return 0;