Revert most changes. Only change web workflow writable

Author: tannewt

extmod/vfs_fat.c

@@ -98,8 +98,8 @@ STATIC mp_obj_t fat_vfs_make_new(const mp_obj_type_t *type, size_t n_args, size_
     return MP_OBJ_FROM_PTR(vfs);
 }
 
-STATIC void filesystem_lock_raise(fs_user_mount_t *vfs) {
-    if (!filesystem_lock(vfs)) {
+STATIC void verify_fs_writable(fs_user_mount_t *vfs) {
+    if (!filesystem_is_writable_by_python(vfs)) {
         mp_raise_OSError(MP_EROFS);
     }
 }
@@ -218,6 +218,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(fat_vfs_ilistdir_obj, 1, 2, fat_vfs_i
 
 STATIC mp_obj_t fat_vfs_remove_internal(mp_obj_t vfs_in, mp_obj_t path_in, mp_int_t attr) {
     mp_obj_fat_vfs_t *self = MP_OBJ_TO_PTR(vfs_in);
+    verify_fs_writable(self);
     const char *path = mp_obj_str_get_str(path_in);
 
     FILINFO fno;
@@ -229,9 +230,7 @@ STATIC mp_obj_t fat_vfs_remove_internal(mp_obj_t vfs_in, mp_obj_t path_in, mp_in
 
     // check if path is a file or directory
     if ((fno.fattrib & AM_DIR) == attr) {
-        filesystem_lock_raise(self);
         res = f_unlink(&self->fatfs, path);
-        filesystem_unlock(self);
 
         if (res != FR_OK) {
             mp_raise_OSError_fresult(res);
@@ -254,18 +253,17 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_2(fat_vfs_rmdir_obj, fat_vfs_rmdir);
 
 STATIC mp_obj_t fat_vfs_rename(mp_obj_t vfs_in, mp_obj_t path_in, mp_obj_t path_out) {
     mp_obj_fat_vfs_t *self = MP_OBJ_TO_PTR(vfs_in);
+    verify_fs_writable(self);
     const char *old_path = mp_obj_str_get_str(path_in);
     const char *new_path = mp_obj_str_get_str(path_out);
 
-    filesystem_lock_raise(self);
     FRESULT res = f_rename(&self->fatfs, old_path, new_path);
     if (res == FR_EXIST) {
         // if new_path exists then try removing it (but only if it's a file)
         fat_vfs_remove_internal(vfs_in, path_out, 0); // 0 == file attribute
         // try to rename again
         res = f_rename(&self->fatfs, old_path, new_path);
     }
-    filesystem_unlock(self);
     if (res == FR_OK) {
         return mp_const_none;
     } else {
@@ -277,10 +275,9 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_3(fat_vfs_rename_obj, fat_vfs_rename);
 
 STATIC mp_obj_t fat_vfs_mkdir(mp_obj_t vfs_in, mp_obj_t path_o) {
     mp_obj_fat_vfs_t *self = MP_OBJ_TO_PTR(vfs_in);
+    verify_fs_writable(self);
     const char *path = mp_obj_str_get_str(path_o);
-    filesystem_lock_raise(self);
     FRESULT res = f_mkdir(&self->fatfs, path);
-    filesystem_unlock(self);
     if (res == FR_OK) {
         return mp_const_none;
     } else {
@@ -466,11 +463,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_3(fat_vfs_utime_obj, vfs_fat_utime);
 
 STATIC mp_obj_t vfs_fat_getreadonly(mp_obj_t self_in) {
     fs_user_mount_t *self = MP_OBJ_TO_PTR(self_in);
-    bool writable = filesystem_lock(self);
-    if (writable) {
-        filesystem_unlock(self);
-    }
-    return mp_obj_new_bool(!writable);
+    return mp_obj_new_bool(!filesystem_is_writable_by_python(self));
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_1(fat_vfs_getreadonly_obj, vfs_fat_getreadonly);
 STATIC const mp_obj_property_t fat_vfs_readonly_obj = {
@@ -494,10 +487,9 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_1(fat_vfs_getlabel_obj, vfs_fat_getlabel);
 
 STATIC mp_obj_t vfs_fat_setlabel(mp_obj_t self_in, mp_obj_t label_in) {
     fs_user_mount_t *self = MP_OBJ_TO_PTR(self_in);
+    verify_fs_writable(self);
     const char *label_str = mp_obj_str_get_str(label_in);
-    filesystem_lock_raise(self);
     FRESULT res = f_setlabel(&self->fatfs, label_str);
-    filesystem_unlock(self);
     if (res != FR_OK) {
         if (res == FR_WRITE_PROTECTED) {
             mp_raise_msg(&mp_type_OSError, MP_ERROR_TEXT("Read-only filesystem"));

extmod/vfs_fat.h

@@ -50,8 +50,6 @@ MP_DECLARE_CONST_FUN_OBJ_3(fat_vfs_open_obj);
 typedef struct _pyb_file_obj_t {
     mp_obj_base_t base;
     FIL fp;
-    // CIRCUITPY-CHANGE: We need to unlock the fs on file close.
-    fs_user_mount_t *fs_mount;
 } pyb_file_obj_t;
 
 #endif  // MICROPY_INCLUDED_EXTMOD_VFS_FAT_H

extmod/vfs_fat_file.c

@@ -127,9 +127,6 @@ STATIC mp_uint_t file_obj_ioctl(mp_obj_t o_in, mp_uint_t request, uintptr_t arg,
     } else if (request == MP_STREAM_CLOSE) {
         // if fs==NULL then the file is closed and in that case this method is a no-op
         if (self->fp.obj.fs != NULL) {
-            if (self->fp.flag & FA_WRITE) {
-                filesystem_unlock(self->fs_mount);
-            }
             FRESULT res = f_close(&self->fp);
             if (res != FR_OK) {
                 *errcode = fresult_to_errno_table[res];
@@ -246,14 +243,13 @@ STATIC mp_obj_t fat_vfs_open(mp_obj_t self_in, mp_obj_t path_in, mp_obj_t mode_i
     }
 
     assert(self != NULL);
-    if ((mode & FA_WRITE) != 0 && !filesystem_lock(self)) {
+    if ((mode & FA_WRITE) != 0 && !filesystem_is_writable_by_python(self)) {
         mp_raise_OSError(MP_EROFS);
     }
 
 
     pyb_file_obj_t *o = m_new_obj_with_finaliser(pyb_file_obj_t);
     o->base.type = type;
-    o->fs_mount = self;
 
     const char *fname = mp_obj_str_get_str(path_in);
     FRESULT res = f_open(&self->fatfs, &o->fp, fname, mode);

supervisor/filesystem.h

@@ -42,6 +42,11 @@ void filesystem_set_internal_concurrent_write_protection(bool concurrent_write_p
 void filesystem_set_writable_by_usb(fs_user_mount_t *vfs, bool usb_writable);
 void filesystem_set_concurrent_write_protection(fs_user_mount_t *vfs, bool concurrent_write_protection);
 
+// Whether user code can modify the filesystem. It doesn't depend on the state
+// of USB. Don't use this for a workflow. In workflows, grab the shared file
+// system lock.
+bool filesystem_is_writable_by_python(fs_user_mount_t *vfs);
+
 // This controls whether USB tries to grab the underlying block device lock
 // during enumeration. If another workflow is modifying the filesystem when this
 // happens, then USB will be readonly.
@@ -52,7 +57,7 @@ fs_user_mount_t *filesystem_for_path(const char *path_in, const char **path_unde
 bool filesystem_native_fatfs(fs_user_mount_t *fs_mount);
 
 // We have two levels of locking. filesystem_* calls grab a shared blockdev lock to allow
-// CircuitPython's fatfs code to edit the blocks. blockdev_* class grab a lock to mutate blocks
+// CircuitPython's fatfs code to edit the blocks. blockdev_* calls grab a lock to mutate blocks
 // directly, excluding any filesystem_* locks.
 
 bool filesystem_lock(fs_user_mount_t *fs_mount);

supervisor/shared/filesystem.c

@@ -208,6 +208,11 @@ void filesystem_set_writable_by_usb(fs_user_mount_t *vfs, bool usb_writable) {
     }
 }
 
+bool filesystem_is_writable_by_python(fs_user_mount_t *vfs) {
+    return (vfs->blockdev.flags & MP_BLOCKDEV_FLAG_CONCURRENT_WRITE_PROTECTED) == 0 ||
+           (vfs->blockdev.flags & MP_BLOCKDEV_FLAG_USB_WRITABLE) == 0;
+}
+
 bool filesystem_is_writable_by_usb(fs_user_mount_t *vfs) {
     return (vfs->blockdev.flags & MP_BLOCKDEV_FLAG_CONCURRENT_WRITE_PROTECTED) == 0 ||
            (vfs->blockdev.flags & MP_BLOCKDEV_FLAG_USB_WRITABLE) != 0;

supervisor/stub/filesystem.c

@@ -55,6 +55,11 @@ void filesystem_set_writable_by_usb(fs_user_mount_t *vfs, bool usb_writable) {
     return;
 }
 
+bool filesystem_is_writable_by_python(fs_user_mount_t *vfs) {
+    (void)vfs;
+    return true;
+}
+
 bool filesystem_is_writable_by_usb(fs_user_mount_t *vfs) {
     return true;
 }
@@ -73,32 +78,3 @@ void filesystem_set_concurrent_write_protection(fs_user_mount_t *vfs, bool concu
 bool filesystem_present(void) {
     return false;
 }
-
-bool filesystem_lock(fs_user_mount_t *fs_mount) {
-    if (fs_mount->lock_count == 0 && !blockdev_lock(fs_mount)) {
-        return false;
-    }
-    fs_mount->lock_count += 1;
-    return true;
-}
-
-void filesystem_unlock(fs_user_mount_t *fs_mount) {
-    assert(fs_mount->lock_count > 0);
-    fs_mount->lock_count -= 1;
-    if (fs_mount->lock_count == 0) {
-        blockdev_unlock(fs_mount);
-    }
-}
-
-bool blockdev_lock(fs_user_mount_t *fs_mount) {
-    if ((fs_mount->blockdev.flags & MP_BLOCKDEV_FLAG_LOCKED) != 0) {
-        return false;
-    }
-    fs_mount->blockdev.flags |= MP_BLOCKDEV_FLAG_LOCKED;
-    return true;
-}
-
-void blockdev_unlock(fs_user_mount_t *fs_mount) {
-    assert(fs_mount->blockdev.flags & MP_BLOCKDEV_FLAG_LOCKED);
-    fs_mount->blockdev.flags &= ~MP_BLOCKDEV_FLAG_LOCKED;
-}