diff --git a/src/fs.cpp b/src/fs.cpp index 01ebe71..7632482 100644 --- a/src/fs.cpp +++ b/src/fs.cpp @@ -254,11 +254,49 @@ kal_uintptr put_name(const char* src, kal_uintptr n, // calling. struct listing { int fd; + int dir; // the descriptor of the directory it was begun upon + listing* next; okl_uptr used; okl_uptr pos; char buf[8192]; }; +// THE ENUMERATIONS NOT YET READ TO THE END, BECAUSE CLOSING THE DIRECTORY MUST +// END THEM. fs.h releases an iterator "by reading past the end or by closing +// the directory that produced it", and the descriptor a listing holds is its +// own, so closing the directory's descriptor does not reach it. Measured before +// this list existed: a C library above that opened a directory larger than its +// buffer, read one entry and closed it, 64 times, was left holding 64 more +// descriptors. +// +// The lock spins rather than suspends, because suspending would make openkal.fs +// depend upon openkal.task, as src/memory.cpp says of its own. +listing* g_listings = nullptr; +volatile int g_listings_held = 0; + +void listings_lock() { + while (__atomic_exchange_n(&g_listings_held, 1, __ATOMIC_ACQUIRE)) okl::sys(okl::nr_sched_yield); +} +void listings_unlock() { __atomic_store_n(&g_listings_held, 0, __ATOMIC_RELEASE); } + +// Ends every listing begun upon `dir', or `one' alone where `dir' is negative. +// What is ended is unlinked under the lock and released after it. +void listings_end(int dir, listing* one) { + listing* done = nullptr; + listings_lock(); + for (listing** p = &g_listings; *p != nullptr;) { + listing* s = *p; + if (dir >= 0 ? s->dir == dir : s == one) { *p = s->next; s->next = done; done = s; } + else p = &s->next; + } + listings_unlock(); + while (done != nullptr) { + listing* s = done; done = s->next; + okl::sys(okl::nr_close, s->fd); + kal_free(s, sizeof(listing), alignof(listing)); + } +} + } // namespace extern "C" { @@ -312,7 +350,11 @@ int kal_fs_open(kal_dir base, const char* name, kal_uintptr len, void kal_fs_close_dir(kal_dir d) { const int fd = okl::unpack(d.h); - if (fd >= 0) { okl::retire(d.h); okl::sys(okl::nr_close, fd); } + if (fd < 0) return; + // Before the close, because after it the number may already name another + // directory whose enumerations are not this one's. + listings_end(fd, nullptr); + okl::retire(d.h); okl::sys(okl::nr_close, fd); } void kal_fs_close_file(kal_file f) { @@ -515,7 +557,8 @@ int kal_fs_list_begin(kal_dir d, kal_uintptr* iter) { if (okl::failed(own)) return okl::translate(own); auto* s = static_cast(kal_alloc(sizeof(listing), alignof(listing))); if (s == nullptr) { okl::sys(okl::nr_close, own); return kal_err_no_memory; } - s->fd = static_cast(own); s->used = 0; s->pos = 0; + s->fd = static_cast(own); s->dir = fd; s->used = 0; s->pos = 0; + listings_lock(); s->next = g_listings; g_listings = s; listings_unlock(); *iter = reinterpret_cast(s); return kal_ok; } @@ -532,8 +575,7 @@ int kal_fs_list_next(kal_dir, kal_uintptr* iter, static_cast(sizeof s->buf)); if (okl::interrupted(r)) continue; if (okl::failed(r) || r == 0) { - okl::sys(okl::nr_close, s->fd); - kal_free(s, sizeof(listing), alignof(listing)); + listings_end(-1, s); *iter = 0; if (name_len) *name_len = 0; return okl::failed(r) ? okl::translate(r) : kal_ok; diff --git a/tests/conformance_fs.cpp b/tests/conformance_fs.cpp index 0213f32..db9e587 100644 --- a/tests/conformance_fs.cpp +++ b/tests/conformance_fs.cpp @@ -95,6 +95,48 @@ int main() { check(kal_fs_open(d, "inner", 5, kal::fs::open::read.bits, &after) != kal_ok, "a released handle is not treated as valid"); + // An enumeration left part-way is ended by closing the directory that + // produced it, which fs.h states. Counted in the kernel's own table, through + // the whole file system this implementation supplies as a directory. + { + kal_dir slash{}; bool have_root = false; + for (kal_uintptr i = 0; i < kal::fs::preopen_count(); ++i) { + char nm[512]; kal_uintptr nl = 0; kal_dir p{}; + if (kal_fs_preopen(i, &p, nm, sizeof nm, &nl) == kal_ok && nl == 1 && nm[0] == '/') { + slash = p; have_root = true; + } + } + auto open_now = [&]() -> int { + kal_dir fds{}; kal_uintptr it = 0; int n = 0; + if (kal_fs_open_dir(slash, "proc/self/fd", 12, &fds) != kal_ok) return -1; + if (kal_fs_list_begin(fds, &it) == kal_ok) { + for (;;) { + char name[512]; kal_uintptr len = 0; int kind = 0; + if (kal_fs_list_next(fds, &it, name, sizeof name, &len, &kind) != kal_ok || it == 0) break; + ++n; + } + } + kal_fs_close_dir(fds); + return n; + }; + if (have_root) { + const int before = open_now(); + check(before >= 0, "the open descriptors are counted through proc/self/fd"); + if (before >= 0) { + for (int i = 0; i < 64; ++i) { + kal_dir again{}; kal_uintptr it = 0; + if (kal_fs_open_dir(root, "okl_dir", 7, &again) != kal_ok) break; + if (kal_fs_list_begin(again, &it) == kal_ok) { + char name[512]; kal_uintptr len = 0; int kind = 0; + kal_fs_list_next(again, &it, name, sizeof name, &len, &kind); + } + kal_fs_close_dir(again); + } + check(open_now() == before, "closing a directory ends an enumeration left part-way"); + } + } + } + check(kal_fs_remove(root, "okl_dir/inner", 13) == kal_ok, "the inner file is removed"); check(kal_fs_remove(root, "okl_dir", 7) == kal_ok, "the directory is removed"); check(kal_fs_remove(root, "okl_probe.txt", 13) == kal_ok, "the file is removed");