0.7.1 --- a bounded transfer waited upon the descriptor below the one it moved

README.md

@@ -8,7 +8,7 @@ for Linux, written on the kernel's own system-call interface.
 openkal = "0.9.0"
 
 [target.'cfg(os = "linux")'.dependencies]
-openkal-linux = "0.7.0"
+openkal-linux = "0.7.1"
 ```
 
 ## Why it does not use a C library

mcpp.toml

@@ -1,7 +1,7 @@
 [package]
 namespace   = "mcpplibs"
 name        = "openkal-linux"
-version     = "0.7.0"
+version     = "0.7.1"
 description = "The reference implementation of openkal for Linux, written on the kernel's own system-call interface so that it can be placed beneath a C library as well as above one."
 license     = "Apache-2.0"
 

src/handle.h

@@ -24,6 +24,18 @@ inline okl_uptr pack(int fd) {
 }
 
 // Returns the descriptor, or -1 if the word does not name a live one.
+//
+// THIS ACCEPTS A WORD THAT WAS NEVER PACKED, AND SILENTLY. A bare descriptor N
+// has the shape of a packed handle naming N-1 whose generation is still zero,
+// so this returns N-1 for it rather than -1. Nothing here can tell the two
+// apart: the word is one machine word and carries no tag.
+//
+// The consequence is that a handle of the OTHER discipline must never reach
+// this function. openkal.stream's handles are bare descriptors (stream.cpp
+// states why), and src/timeout.cpp used to pass one here and wait upon the
+// descriptor below the one it then transferred upon. Owned handles --- kal_file,
+// kal_dir, kal_net_listener, kal_net_conn, kal_datagram --- are the whole of
+// this function's domain.
 inline int unpack(okl_uptr h) {
     const int fd = static_cast<int>(h & 0xffffffffu) - 1;
     if (fd < 0 || fd >= kMaxDescriptor) return -1;

src/timeout.cpp

@@ -52,6 +52,33 @@ int await(int fd, short events, kal_u64 ns) {
     }
 }
 
+// A STREAM HANDLE IS A DESCRIPTOR AND IS NOT DECODED. stream.cpp states it in
+// terms, kal_stream_read and kal_stream_write take it as one, and so must the
+// wait that precedes them: the wait and the transfer that follows it have to
+// name the same object or the wait answers about something else.
+//
+// THIS FILE USED TO DECODE IT AS AN OWNED HANDLE, AND THE DECODE SUCCEEDED.
+// handle.h packs an owned handle as (generation << 32) | (fd + 1), so a bare
+// descriptor N has exactly the shape of a packed handle naming N-1, and
+// okl::unpack accepts it whenever generation N-1 is still zero -- which is
+// every index at which no owned handle has yet been released. The wait was
+// therefore performed upon descriptor N-1 and the transfer upon N.
+//
+// The former reading tested `unpack` and fell back when it failed, on the
+// stated ground that the standard streams are not packed. That ground is
+// correct and the conclusion drawn from it was not: NO stream handle is packed
+// here, and of the three standard ones only kal_stdin, whose handle is zero,
+// fails to decode. kal_stdout decoded to descriptor 0 and kal_stderr to 1.
+//
+// What a caller observed was an expiry for a stream that had bytes waiting, or
+// a wait without bound inside an operation that states one, according to what
+// happened to occupy the descriptor below. Both are functions of the process's
+// descriptor history, so the same program answered differently when run alone
+// and when run after something else -- and the index healed for good once an
+// owned handle at N-1 had been released, because that advances the generation
+// and makes the decode fail correctly.
+int stream_fd(kal_stream s) { return static_cast<int>(s.h); }
+
 }  // namespace
 
 extern "C" {
@@ -61,26 +88,24 @@ kal_intptr kal_timeout_read(kal_stream s, void* buf, kal_uintptr len, kal_u64 ns
     // would turn a call that always succeeds into one that can expire.
     if (len == 0) return 0;
 
-    const int fd = okl::unpack(s.h);
-    // THE STANDARD STREAMS ARE NOT PACKED HANDLES. openkal.stream reports them
-    // as the descriptors themselves, so a word that does not unpack is taken to
-    // be one of those rather than being refused.
-    const int use = (fd >= 0) ? fd : static_cast<int>(s.h);
-
-    if (const int rc = await(use, okl::poll_in, ns); rc != kal_ok) return -rc;
+    if (const int rc = await(stream_fd(s), okl::poll_in, ns); rc != kal_ok) return -rc;
     return kal_stream_read(s, buf, len);
 }
 
 kal_intptr kal_timeout_write(kal_stream s, const void* buf, kal_uintptr len, kal_u64 ns) {
     if (len == 0) return 0;
 
-    const int fd  = okl::unpack(s.h);
-    const int use = (fd >= 0) ? fd : static_cast<int>(s.h);
-
-    if (const int rc = await(use, okl::poll_out, ns); rc != kal_ok) return -rc;
+    if (const int rc = await(stream_fd(s), okl::poll_out, ns); rc != kal_ok) return -rc;
     return kal_stream_write(s, buf, len);
 }
 
+// THE TWO OPERATIONS BELOW DO DECODE, AND THAT IS NOT AN INCONSISTENCY WITH THE
+// TWO ABOVE. A listener and a datagram are owned: their handles are made by
+// okl::pack and released by okl::retire, so the decode is the operation that
+// recovers the descriptor and its failure is how a released handle is refused.
+// A stream is borrowed and carries no generation. The four call sites divide
+// exactly along that line, and the two that were wrong were the two that had a
+// borrowed handle in hand.
 int kal_timeout_accept(kal_net_listener l, kal_u64 ns, kal_net_conn* out) {
     if (out == nullptr) return kal_err_invalid;
     const int fd = okl::unpack(l.h);

tests/conformance_v08.cpp

@@ -190,6 +190,75 @@ void space_section() {
     kal_process_close(p.p);
 }
 
+// A BOUNDED TRANSFER WAITS UPON THE STREAM IT THEN TRANSFERS UPON.
+//
+// This is the observation that was missing while the two operations it examines
+// waited upon the descriptor below the one they transferred upon. Nothing else
+// in this file reached them: the two observations in timeout_section below are
+// of an accept, whose handle is owned and was decoded correctly, and of a
+// zero-length write, which returns before it waits.
+//
+// WHY IT MAKES SIXTEEN CHANNELS AND NOT ONE. Under the defect the wait was
+// performed upon descriptor N-1, and whether that expires depends on what
+// occupies N-1. For a channel made after another, N-1 is the previous channel's
+// writing end, upon which input is never reported, so the read expires while
+// bytes sit in the stream that was asked for. One channel would be answering
+// about whichever descriptor happened to precede it; sixteen make the answer a
+// property of the implementation rather than of the process that ran it.
+//
+// AND WHY IT RUNS FIRST. The mistaken decode succeeded only while the
+// generation recorded for N-1 was still zero, so it corrected itself for every
+// index at which an owned handle had already been released. Placed after the
+// sections that create and release listeners and datagrams, this would have
+// examined reused descriptors and held upon the defect.
+//
+// The count is reported rather than the first failure. Sixteen distinguishes a
+// correct implementation from one that answers about a neighbouring descriptor;
+// the first failure alone would not say which.
+void timeout_stream_section() {
+    constexpr int n = 16;
+    kal_stream mine[n]{}, theirs[n]{};
+    int made = 0;
+
+    for (; made < n; ++made) {
+        if (kal_process_channel(&mine[made], &theirs[made]) != kal_ok) break;
+        const char byte = 'x';
+        if (kal_stream_write(theirs[made], &byte, 1) != 1) break;
+    }
+    check(made == n, "sixteen channels are created and each is written to");
+
+    int transferred = 0;
+    for (int i = 0; i < made; ++i) {
+        char buf[1] = {};
+        // One millisecond. Every one of these streams has a byte waiting, so a
+        // correct implementation does not reach the bound at all; the bound is
+        // present so that a wait upon the wrong descriptor ends the run rather
+        // than hanging it.
+        if (kal::timeout::read(mine[i], buf, 1, 1000000) == 1 && buf[0] == 'x')
+            ++transferred;
+    }
+    if (transferred != made)
+        std::printf("  %d of %d bounded reads transferred\n", transferred, made);
+    check(transferred == made,
+          "a bounded read of a stream that has bytes waiting transfers them");
+
+    // The mirror. A stream with nothing in it must expire, and must expire
+    // rather than transfer a byte belonging to another stream.
+    kal_stream empty_mine{}, empty_theirs{};
+    if (kal_process_channel(&empty_mine, &empty_theirs) == kal_ok) {
+        char buf[1] = {};
+        check(kal::timeout::read(empty_mine, buf, 1, 1000000) == -kal_err_again,
+              "a bounded read of a stream with nothing waiting expires");
+        kal_process_channel_close(empty_theirs);
+        kal_process_channel_close(empty_mine);
+    }
+
+    for (int i = 0; i < made; ++i) {
+        kal_process_channel_close(theirs[i]);
+        kal_process_channel_close(mine[i]);
+    }
+}
+
 // openkal.timeout
 void timeout_section() {
     check(kal_timeout_granularity() > 0,
@@ -257,6 +326,12 @@ void process_additions_section() {
 }  // namespace
 
 int main() {
+    // FIRST, AND THE ORDER IS LOAD-BEARING. The comment above this section
+    // records why: it examines a defect that corrects itself for any descriptor
+    // index at which an owned handle has already been released, so running it
+    // after the sections that make and release listeners would hold upon the
+    // defect it exists to detect.
+    timeout_stream_section();
     process_additions_section();
     terminal_section();
     net_section();