extmod/vfs_posix_file: Implement finaliser for files.

Prevent handle leaks when file objects aren't closed explicitly and
fix some MICROPY_CPYTHON_COMPAT issues: this wasn't properly adhered
to because #ifdef was used so it was always on, and closing files
multiple times should be avoided unconditionally.
pull/8970/head
stijn 2022-09-12 14:45:39 +02:00 zatwierdzone przez Damien George
rodzic fb77be1506
commit 9ae8d38204
3 zmienionych plików z 38 dodań i 7 usunięć

Wyświetl plik

@ -46,7 +46,7 @@ typedef struct _mp_obj_vfs_posix_file_t {
int fd; int fd;
} mp_obj_vfs_posix_file_t; } mp_obj_vfs_posix_file_t;
#ifdef MICROPY_CPYTHON_COMPAT #if MICROPY_CPYTHON_COMPAT
STATIC void check_fd_is_open(const mp_obj_vfs_posix_file_t *o) { STATIC void check_fd_is_open(const mp_obj_vfs_posix_file_t *o) {
if (o->fd < 0) { if (o->fd < 0) {
mp_raise_ValueError(MP_ERROR_TEXT("I/O operation on closed file")); mp_raise_ValueError(MP_ERROR_TEXT("I/O operation on closed file"));
@ -63,7 +63,7 @@ STATIC void vfs_posix_file_print(const mp_print_t *print, mp_obj_t self_in, mp_p
} }
mp_obj_t mp_vfs_posix_file_open(const mp_obj_type_t *type, mp_obj_t file_in, mp_obj_t mode_in) { mp_obj_t mp_vfs_posix_file_open(const mp_obj_type_t *type, mp_obj_t file_in, mp_obj_t mode_in) {
mp_obj_vfs_posix_file_t *o = m_new_obj(mp_obj_vfs_posix_file_t); mp_obj_vfs_posix_file_t *o = m_new_obj_with_finaliser(mp_obj_vfs_posix_file_t);
const char *mode_s = mp_obj_str_get_str(mode_in); const char *mode_s = mp_obj_str_get_str(mode_in);
int mode_rw = 0, mode_x = 0; int mode_rw = 0, mode_x = 0;
@ -185,12 +185,12 @@ STATIC mp_uint_t vfs_posix_file_ioctl(mp_obj_t o_in, mp_uint_t request, uintptr_
return 0; return 0;
} }
case MP_STREAM_CLOSE: case MP_STREAM_CLOSE:
MP_THREAD_GIL_EXIT(); if (o->fd >= 0) {
close(o->fd); MP_THREAD_GIL_EXIT();
MP_THREAD_GIL_ENTER(); close(o->fd);
#ifdef MICROPY_CPYTHON_COMPAT MP_THREAD_GIL_ENTER();
}
o->fd = -1; o->fd = -1;
#endif
return 0; return 0;
case MP_STREAM_GET_FILENO: case MP_STREAM_GET_FILENO:
return o->fd; return o->fd;
@ -237,6 +237,7 @@ STATIC const mp_rom_map_elem_t vfs_posix_rawfile_locals_dict_table[] = {
{ MP_ROM_QSTR(MP_QSTR_tell), MP_ROM_PTR(&mp_stream_tell_obj) }, { MP_ROM_QSTR(MP_QSTR_tell), MP_ROM_PTR(&mp_stream_tell_obj) },
{ MP_ROM_QSTR(MP_QSTR_flush), MP_ROM_PTR(&mp_stream_flush_obj) }, { MP_ROM_QSTR(MP_QSTR_flush), MP_ROM_PTR(&mp_stream_flush_obj) },
{ MP_ROM_QSTR(MP_QSTR_close), MP_ROM_PTR(&mp_stream_close_obj) }, { MP_ROM_QSTR(MP_QSTR_close), MP_ROM_PTR(&mp_stream_close_obj) },
{ MP_ROM_QSTR(MP_QSTR___del__), MP_ROM_PTR(&mp_stream_close_obj) },
{ MP_ROM_QSTR(MP_QSTR___enter__), MP_ROM_PTR(&mp_identity_obj) }, { MP_ROM_QSTR(MP_QSTR___enter__), MP_ROM_PTR(&mp_identity_obj) },
{ MP_ROM_QSTR(MP_QSTR___exit__), MP_ROM_PTR(&vfs_posix_file___exit___obj) }, { MP_ROM_QSTR(MP_QSTR___exit__), MP_ROM_PTR(&vfs_posix_file___exit___obj) },
}; };

Wyświetl plik

@ -1,6 +1,7 @@
# Test for VfsPosix # Test for VfsPosix
try: try:
import gc
import uos import uos
uos.VfsPosix uos.VfsPosix
@ -51,6 +52,34 @@ f = open(temp_dir + "/test", "r")
print(f.read()) print(f.read())
f.close() f.close()
# file finaliser, also see vfs_fat_finaliser.py
names = [temp_dir + "/x%d" % i for i in range(4)]
basefd = temp_dir + "/nextfd1"
nextfd = temp_dir + "/nextfd2"
with open(basefd, "w") as f:
base_file_no = f.fileno()
for i in range(1024): # move GC head forwards by allocating a lot of single blocks
[]
def write_files_without_closing():
for n in names:
open(n, "w").write(n)
sorted(list(range(128)), key=lambda x: x) # use up Python and C stack so f is really gone
write_files_without_closing()
gc.collect()
with open(nextfd, "w") as f:
next_file_no = f.fileno()
print("next_file_no <= base_file_no", next_file_no <= base_file_no)
for n in names + [basefd, nextfd]:
uos.remove(n)
# rename # rename
uos.rename(temp_dir + "/test", temp_dir + "/test2") uos.rename(temp_dir + "/test", temp_dir + "/test2")
print(uos.listdir(temp_dir)) print(uos.listdir(temp_dir))

Wyświetl plik

@ -4,6 +4,7 @@ True
<class 'list'> <class 'list'>
<io.TextIOWrapper 2> <io.TextIOWrapper 2>
hello hello
next_file_no <= base_file_no True
['test2'] ['test2']
['test2'] ['test2']
<class 'tuple'> <class 'tuple'>