From cd92b326220f1839a0885ad3d1f253ec3b3cbd18 Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Fri, 6 Dec 2019 17:11:17 +0100 Subject: [PATCH] libc: close all open FDs on exit This is important to issue sync requests for written-to files. As the closing must be performed by an atexit handler, it happens at a time _after_ libc plugins are destructed. Consequently an FD allocated by such a plugin results in a close error, which in turn, does not destruct the FD. We ultimatedly end up in an infinte loop of re-attempting the close. For this reason, the patch changes 'close' to be robust against this special case. This is generally not a problem because libc plugins are phased out. However, at present, the libc_noux plugin is still important. With the changed 'close' in place, there occurred an error message "Error: close: close not implemented" at the exit of each noux program. This patch removes the error printing from the libc plugin mechansim to avoid this noise. The error messages are not important anyway because the deprecation of the libc plugin interface. Issue #3578 --- repos/libports/include/libc-plugin/fd_alloc.h | 6 ++++++ repos/libports/src/lib/libc/exit.cc | 5 +++++ repos/libports/src/lib/libc/fd_alloc.cc | 12 ++++++++++++ repos/libports/src/lib/libc/file_operations.cc | 11 ++++++++--- repos/libports/src/lib/libc/kernel.cc | 13 +++++++++++++ repos/libports/src/lib/libc/plugin.cc | 1 - 6 files changed, 44 insertions(+), 4 deletions(-) diff --git a/repos/libports/include/libc-plugin/fd_alloc.h b/repos/libports/include/libc-plugin/fd_alloc.h index f046636b51..593529f31f 100644 --- a/repos/libports/include/libc-plugin/fd_alloc.h +++ b/repos/libports/include/libc-plugin/fd_alloc.h @@ -112,6 +112,12 @@ namespace Libc { */ File_descriptor *any_cloexec_libc_fd(); + /** + * Return file-descriptor ID of any open file, or -1 if no file is + * open + */ + int any_open_fd(); + void generate_info(Genode::Xml_generator &); }; diff --git a/repos/libports/src/lib/libc/exit.cc b/repos/libports/src/lib/libc/exit.cc index 926de9c5ae..bf178584f0 100644 --- a/repos/libports/src/lib/libc/exit.cc +++ b/repos/libports/src/lib/libc/exit.cc @@ -14,6 +14,9 @@ #include #include +/* libc-internal includes */ +#include + extern void genode_exit(int status) __attribute__((noreturn)); extern "C" void _exit(int status) @@ -31,6 +34,8 @@ extern "C" { void exit(int status) { + using namespace Libc; + if (__cleanup) (*__cleanup)(); diff --git a/repos/libports/src/lib/libc/fd_alloc.cc b/repos/libports/src/lib/libc/fd_alloc.cc index 57e0c4c0f3..27dd9e4738 100644 --- a/repos/libports/src/lib/libc/fd_alloc.cc +++ b/repos/libports/src/lib/libc/fd_alloc.cc @@ -125,6 +125,18 @@ File_descriptor *File_descriptor_allocator::any_cloexec_libc_fd() } +int File_descriptor_allocator::any_open_fd() +{ + Lock::Guard guard(_lock); + + int result = -1; + _id_space.apply_any([&] (File_descriptor &fd) { + result = fd.libc_fd; }); + + return result; +} + + void File_descriptor_allocator::generate_info(Xml_generator &xml) { Lock::Guard guard(_lock); diff --git a/repos/libports/src/lib/libc/file_operations.cc b/repos/libports/src/lib/libc/file_operations.cc index 4b467baa79..bb4d274adc 100644 --- a/repos/libports/src/lib/libc/file_operations.cc +++ b/repos/libports/src/lib/libc/file_operations.cc @@ -234,9 +234,14 @@ extern "C" int chdir(const char *path) __SYS_(int, close, (int libc_fd), { File_descriptor *fd = file_descriptor_allocator()->find_by_libc_fd(libc_fd); - return (!fd || !fd->plugin) - ? Errno(EBADF) - : fd->plugin->close(fd); + + if (!fd) + return Errno(EBADF); + + if (!fd->plugin || fd->plugin->close(fd) != 0) + file_descriptor_allocator()->free(fd); + + return 0; }) diff --git a/repos/libports/src/lib/libc/kernel.cc b/repos/libports/src/lib/libc/kernel.cc index f04160a5ff..222a91b08f 100644 --- a/repos/libports/src/lib/libc/kernel.cc +++ b/repos/libports/src/lib/libc/kernel.cc @@ -359,10 +359,23 @@ void Libc::execute_in_application_context(Application_code &app_code) } +static void close_file_descriptors_on_exit() +{ + for (;;) { + int const fd = Libc::file_descriptor_allocator()->any_open_fd(); + if (fd == -1) + break; + close(fd); + } +} + + Libc::Kernel::Kernel(Genode::Env &env, Genode::Allocator &heap) : _env(env), _heap(heap) { + atexit(close_file_descriptors_on_exit); + init_pthread_support(env, *this, *this); _env.ep().register_io_progress_handler(*this); diff --git a/repos/libports/src/lib/libc/plugin.cc b/repos/libports/src/lib/libc/plugin.cc index b4932944b5..75693078df 100644 --- a/repos/libports/src/lib/libc/plugin.cc +++ b/repos/libports/src/lib/libc/plugin.cc @@ -155,7 +155,6 @@ bool Plugin::supports_mmap() #define DUMMY(ret_type, ret_val, name, args) \ ret_type Plugin::name args \ { \ - error(__func__, ": " #name " not implemented"); \ return ret_val; \ }