From 17075b5a0120a0cf9791cf49c2c730110e44a6d1 Mon Sep 17 00:00:00 2001 From: Sunrisepeak Date: Thu, 12 Nov 2026 18:57:11 +0900 Subject: [PATCH] 0.23.1: a handle is retired before its descriptor is closed; one release closes; a released handle's stream is no stream --- README.md | 22 +++- mcpp.toml | 2 +- src/datagram.cpp | 7 +- src/fs.cpp | 10 +- src/handle.h | 47 ++++++- src/net.cpp | 17 +-- ...dle_is_stale_before_its_number_is_free.cpp | 122 ++++++++++++++++++ 7 files changed, 194 insertions(+), 33 deletions(-) create mode 100644 tests/a_released_handle_is_stale_before_its_number_is_free.cpp diff --git a/README.md b/README.md index 0a68801..8cfa4f7 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ written on the kernel's own calls. openkal = "0.25.0" [target.'cfg(os = "macos")'.dependencies] -openkal-macos = "0.23.0" +openkal-macos = "0.23.1" ``` Its purpose is as much to test the specification as to be used. A specification @@ -183,6 +183,26 @@ same image parses as fast on Linux arm64. Since 0.17.1 it is the same kernel ide this system's library keeps for each thread (`pthread_threadid_np`, a name no C library in this ecosystem defines). +## What 0.23.1 corrects + +**A handle is retired before its descriptor is closed.** A connection, a listener +and a datagram socket were closed first and retired second. The kernel gives a +closed number to the next open in any context, so a file opened by another context +between the two took the number with the generation the socket's handle still had, +and the retire then made the file's handle stale: its stream was answered as zero, +which is standard input, its reads returned nothing of the file, and its descriptor +was never closed (openkal-linux#53; this implementation has the same construction). +Every release is now one function, which retires and then closes. + +**One release closes.** The generation is advanced by a compare-and-exchange from +the value the handle carries, so a second release of the same word, in the same +context or another, closes nothing. It is read and advanced through the atomic +operations. + +**A released handle's stream is no stream.** `kal_fs_stream` and `kal_net_stream` +answer a handle that names no live descriptor with a word no descriptor has, and a +transfer upon it reports `kal_err_invalid`. They answered zero. + ## What 0.23 adds (openkal 0.25) **A second name for a file** (`kal_fs_hard_link`) is `linkat` with no flags, so a diff --git a/mcpp.toml b/mcpp.toml index 8c7e35a..b5bf5e3 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -1,7 +1,7 @@ [package] namespace = "mcpplibs" name = "openkal-macos" -version = "0.23.0" +version = "0.23.1" description = "An implementation of openkal for macOS, written on the kernel's own calls. Its purpose is as much to test the specification as to be used." license = "Apache-2.0" diff --git a/src/datagram.cpp b/src/datagram.cpp index 3443122..8ecb1b9 100644 --- a/src/datagram.cpp +++ b/src/datagram.cpp @@ -137,12 +137,7 @@ kal_intptr kal_datagram_recv_from(kal_datagram d, void* buf, kal_uintptr len, } } -void kal_datagram_close(kal_datagram d) { - const int fd = fd_of(d); - if (fd < 0) return; - okm::sys(okm::nr_close, fd); - okm::retire(d.h); -} +void kal_datagram_close(kal_datagram d) { okm::release(d.h); } // Broadcast is not claimed. The kernel provides it only after SO_BROADCAST has // been set, and this interface has no operation that would set it; a word diff --git a/src/fs.cpp b/src/fs.cpp index f5bf561..a9402eb 100644 --- a/src/fs.cpp +++ b/src/fs.cpp @@ -517,19 +517,15 @@ void kal_fs_close_dir(kal_dir d) { // directory whose enumerations are not this one's. listings_end(fd); okm::rights::forget(fd); - okm::retire(d.h); okm::sys(okm::nr_close, fd); + okm::release(d.h); } -void kal_fs_close_file(kal_file f) { - const int fd = okm::unpack(f.h); - if (fd >= 0) { okm::retire(f.h); okm::sys(okm::nr_close, fd); } -} +void kal_fs_close_file(kal_file f) { okm::release(f.h); } kal_uintptr kal_fs_max_name(void) { return okm::max_name; } kal_stream kal_fs_stream(kal_file f) { - const int fd = okm::unpack(f.h); - return kal_stream{ fd < 0 ? 0u : static_cast(fd) }; + return kal_stream{ okm::stream_word(f.h) }; } int kal_fs_seek(kal_file f, kal_i64 offset, int whence, kal_u64* result) { diff --git a/src/handle.h b/src/handle.h index e0ac745..efb65c7 100644 --- a/src/handle.h +++ b/src/handle.h @@ -17,9 +17,16 @@ inline unsigned* generations() { return g; } +// A generation is read by the context that opens a descriptor and advanced by +// the one that releases it, and the two are different contexts whenever a +// number is reused across them, so both go through the atomic operations. +inline unsigned generation(int fd) { + return __atomic_load_n(&generations()[fd], __ATOMIC_RELAXED); +} + inline okm_uptr pack(int fd) { if (fd < 0 || fd >= kMaxDescriptor) return 0; - return (static_cast(generations()[fd]) << 32) + return (static_cast(generation(fd)) << 32) | (static_cast(fd) + 1u); } @@ -39,13 +46,45 @@ inline okm_uptr pack(int fd) { inline int unpack(okm_uptr h) { const int fd = static_cast(h & 0xffffffffu) - 1; if (fd < 0 || fd >= kMaxDescriptor) return -1; - if (static_cast(h >> 32) != generations()[fd]) return -1; + if (static_cast(h >> 32) != generation(fd)) return -1; return fd; } -inline void retire(okm_uptr h) { +// RETIRE, THEN CLOSE. The kernel gives a closed number to the next open in any +// context. A handle retired after its descriptor was closed leaves a window in +// which another context opens the number, packs it with the generation the +// handle being released still has, and is then left with a handle this retire +// makes stale: its stream reads and writes nothing of its own, and its +// descriptor is never closed (openkal-linux#53, the same construction). Every +// release in this implementation is `release(h)`, which keeps the order in one +// place. +// +// One release of a handle advances its generation; a second release of the +// same word, in this context or another, finds it advanced and closes nothing, +// so it cannot close a descriptor opened since. +inline bool retire(okm_uptr h) { + const int fd = static_cast(h & 0xffffffffu) - 1; + if (fd < 0 || fd >= kMaxDescriptor) return false; + unsigned held = static_cast(h >> 32); + return __atomic_compare_exchange_n(&generations()[fd], &held, held + 1u, false, + __ATOMIC_RELAXED, __ATOMIC_RELAXED); +} + +inline void release(okm_uptr h) { + const int fd = static_cast(h & 0xffffffffu) - 1; + if (retire(h)) sys(nr_close, fd); +} + +// What a stream enquiry answers for a handle that does not name a live +// descriptor. A stream is a bare descriptor here (stream.cpp), so zero is +// standard input: a released handle answered with zero was read from and +// written to as standard input. No descriptor has this value, and every +// transfer upon it reports `kal_err_invalid`. +inline constexpr okm_uptr kNoStream = ~static_cast(0); + +inline okm_uptr stream_word(okm_uptr h) { const int fd = unpack(h); - if (fd >= 0) ++generations()[fd]; + return fd < 0 ? kNoStream : static_cast(fd); } } // namespace okm diff --git a/src/net.cpp b/src/net.cpp index 692fa67..68f6b82 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -200,8 +200,7 @@ kal_stream kal_net_stream(kal_net_conn c) { // The bare descriptor, for the reason kal_fs_stream gives: openkal.stream's // operations take whatever the environment's transfer calls take, and a // packed word is not that. - const int fd = fd_of(c); - return kal_stream{ fd < 0 ? 0u : static_cast(fd) }; + return kal_stream{ okm::stream_word(c.h) }; } int kal_net_peer(kal_net_conn c, kal_endpoint* out) { @@ -237,19 +236,9 @@ int kal_net_shutdown(kal_net_conn c, int direction) { return kal_ok; } -void kal_net_close(kal_net_conn c) { - const int fd = fd_of(c); - if (fd < 0) return; - okm::sys(okm::nr_close, fd); - okm::retire(c.h); -} +void kal_net_close(kal_net_conn c) { okm::release(c.h); } -void kal_net_close_listener(kal_net_listener l) { - const int fd = fd_of(l); - if (fd < 0) return; - okm::sys(okm::nr_close, fd); - okm::retire(l.h); -} +void kal_net_close_listener(kal_net_listener l) { okm::release(l.h); } // Both positions hold on this kernel: it speaks IPv6 and its `shutdown' ends // transfer in one direction while the other continues. diff --git a/tests/a_released_handle_is_stale_before_its_number_is_free.cpp b/tests/a_released_handle_is_stale_before_its_number_is_free.cpp new file mode 100644 index 0000000..33d6c73 --- /dev/null +++ b/tests/a_released_handle_is_stale_before_its_number_is_free.cpp @@ -0,0 +1,122 @@ +// A handle is retired before its descriptor is closed (0.23.1; openkal-linux#53, the same construction). +// +// The kernel gives a closed number to the next open in any context. Until +// 0.23.1 a connection, a listener and a datagram socket were closed first and +// retired second: a file opened by another context between the two took the +// number with the generation the socket's handle still had, and the retire then +// made the file's handle stale. Its stream was answered as zero, standard +// input, so its reads returned nothing of the file and its descriptor was never +// closed. Measured by the report on four processors: about 150 such files a +// second. +// +// One context accepts connections and closes them, one connects and closes, and +// this one opens, reads and closes a file. Every read returns the file. +#include +#include +#include +#include +import openkal.types; +import openkal.fs; +import openkal.net; +import openkal.stream; +import mcpplibs.mtest; + +using namespace mcpplibs::mtest; + +namespace { + +// The working directory, which is the first directory the environment supplies. +kal_dir root() { + kal_dir d{}; char n[4096]; kal_uintptr l = 0; + return kal_fs_preopen(0, &d, n, sizeof n, &l) == kal_ok ? d : kal_dir{}; +} + +constexpr char kName[] = "okl_retire_probe.txt"; +constexpr char kText[] = "the file's bytes"; // 16 + +} // namespace + +UTest a_released_handle_is_stale_before_its_number_is_free{ + "a_released_handle_is_stale_before_its_number_is_free", [] { + const kal_dir dir = root(); + expect(fatal(dir.h != 0)) << "the environment supplies the working directory"; + { + kal_file f{}; + expect(fatal(kal_fs_open(dir, kName, sizeof kName - 1, + (kal::fs::open::write | kal::fs::open::create | kal::fs::open::truncate).bits, + &f) == kal_ok)); + expect(fatal(kal_stream_write(kal_fs_stream(f), kText, 16) == 16)); + kal_fs_close_file(f); + } + + kal_endpoint any{}; + any.addr[0] = 127; any.addr[3] = 1; any.addr_len = 4; + kal_net_listener lis{}; + expect(fatal(kal_net_listen(&any, &lis) == kal_ok)); + kal_endpoint at{}; + expect(fatal(kal_net_listener_local(lis, &at) == kal_ok)); + + std::atomic stop{false}; + std::atomic served{0}; + std::thread accepting([&] { + for (;;) { + kal_net_conn c{}; + if (kal_net_accept(lis, &c) != kal_ok) break; + kal_stream_write(kal_net_stream(c), "okl", 3); + kal_net_close(c); + served.fetch_add(1, std::memory_order_relaxed); + if (stop.load(std::memory_order_relaxed)) break; + } + }); + std::thread connecting([&] { + while (!stop.load(std::memory_order_relaxed)) { + kal_net_conn c{}; + if (kal_net_connect(&at, &c) == kal_ok) kal_net_close(c); + } + // One more, so the accepting context leaves its wait and sees the word. + kal_net_conn last{}; + if (kal_net_connect(&at, &last) == kal_ok) kal_net_close(last); + }); + + long opened = 0, stale = 0, short_reads = 0; + const auto until = std::chrono::steady_clock::now() + std::chrono::seconds(3); + while (std::chrono::steady_clock::now() < until) { + for (int i = 0; i < 256; ++i) { + kal_file f{}; + if (kal_fs_open(dir, kName, sizeof kName - 1, kal::fs::open::read.bits, &f) != kal_ok) continue; + ++opened; + char buf[32]{}; + const kal_intptr n = kal_stream_read(kal_fs_stream(f), buf, sizeof buf); + if (n < 0) ++stale; + else if (n != 16 || std::memcmp(buf, kText, 16) != 0) ++short_reads; + kal_fs_close_file(f); + } + } + stop.store(true, std::memory_order_relaxed); + connecting.join(); + accepting.join(); + kal_net_close_listener(lis); + + expect(gt(opened, 0L)); + expect(gt(served.load(), 0L)) << "no connection was closed beside the opens; the test measured nothing"; + expect(eq(stale, 0L)) << "files whose handle was stale when its stream was taken, of " << opened; + expect(eq(short_reads, 0L)) << "files whose read was not the file, of " << opened; + + // A released handle's stream is no stream: a transfer upon it is refused, + // and is not a transfer upon standard input. + kal_file f{}; + expect(fatal(kal_fs_open(dir, kName, sizeof kName - 1, kal::fs::open::read.bits, &f) == kal_ok)); + kal_fs_close_file(f); + char one = 0; + expect(eq(kal_stream_read(kal_fs_stream(f), &one, 1), static_cast(-kal_err_invalid))); + // And a second release of it closes nothing: the number may already be + // another's. + kal_file g{}; + expect(fatal(kal_fs_open(dir, kName, sizeof kName - 1, kal::fs::open::read.bits, &g) == kal_ok)); + kal_fs_close_file(f); + char buf[32]{}; + expect(eq(kal_stream_read(kal_fs_stream(g), buf, sizeof buf), static_cast(16))); + kal_fs_close_file(g); + + kal_fs_remove(dir, kName, sizeof kName - 1); +}};