[tmp] openkal 0.16 draft: least stack size, PASS_INPUT, enumeration release (verification only)

README.md

@@ -4,10 +4,10 @@ An implementation of [openkal](https://github.com/mcpplibs/openkal) for Windows.
 
 ```toml
 [dependencies]
-openkal = "0.15.0"
+openkal = "0.16.0"
 
 [target.'cfg(windows)'.dependencies]
-openkal-windows = "0.11.0"
+openkal-windows = "0.12.0"
 ```
 
 Its purpose is as much to test the specification as to be used. openkal was

mcpp.toml

@@ -1,7 +1,7 @@
 [package]
 namespace   = "mcpplibs"
 name        = "openkal-windows"
-version     = "0.11.0"
+version     = "0.12.0"
 description = "An implementation of openkal for Windows, written on the Win32 interfaces and the object manager beneath them, using no C runtime symbol."
 license     = "Apache-2.0"
 
@@ -62,7 +62,7 @@ provides-interfaces = [
 ]
 
 [dependencies]
-openkal = "0.15.0"
+openkal = "0.16.0"
 
 # The package contributes definitions and no modules. The interface it
 # implements is declared by the specification package, which this package

src/fs.cpp

@@ -230,6 +230,10 @@ void fill_absent(kal_node_info* out) {
 // each other's entries.
 struct listing {
     void*    handle;
+    void*    dir;      // the handle of the directory it was begun upon
+    listing* next;
+    int      busy;     // a context is inside kal_fs_list_next with it
+    int      doomed;   // its directory was closed while it was busy
     okw_uptr used;
     okw_uptr at;
     bool     first;
@@ -248,6 +252,72 @@ struct listing {
     char     reported[512];
 };
 
+// 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 handle a listing holds is its own,
+// so closing the directory's handle does not reach it. Measured on the Linux
+// implementation, which had the same shape (mcpplibs/openkal-linux#34): a C
+// library above that stopped reading a directory early kept a handle for every
+// directory it closed.
+//
+// A slim lock, because it is one pointer, all-zero is unlocked, and it needs
+// nothing from openkal.task.
+listing* g_listings = nullptr;
+SRWLOCK_ g_listings_lock{};
+
+void listing_release(listing* s) {
+    okw::NtClose(s->handle);
+    kal_free(s, sizeof(listing), alignof(listing));
+}
+
+// Ends every listing begun upon `dir'. A listing another context is reading is
+// marked rather than released, and that context releases it on its way out:
+// clause 6.6 requires that concurrent operations upon one handle not damage
+// this implementation's state, and freeing a buffer the system is writing into
+// would.
+void listings_end(void* dir) {
+    listing* done = nullptr;
+    AcquireSRWLockExclusive(&g_listings_lock);
+    for (listing** p = &g_listings; *p != nullptr;) {
+        listing* s = *p;
+        if (s->dir != dir || s->doomed) { p = &s->next; continue; }
+        if (s->busy) { s->doomed = 1; p = &s->next; continue; }
+        *p = s->next; s->next = done; done = s;
+    }
+    ReleaseSRWLockExclusive(&g_listings_lock);
+    while (done != nullptr) { listing* s = done; done = s->next; listing_release(s); }
+}
+
+// Takes `s' for the calling context, or reports that it is not an open listing
+// of `dir'. The word is found among the open listings rather than read through,
+// so a stale iterator is refused rather than followed.
+bool listing_take(listing* s, void* dir) {
+    for (;;) {
+        AcquireSRWLockExclusive(&g_listings_lock);
+        listing* p = g_listings;
+        while (p != nullptr && p != s) p = p->next;
+        if (p == nullptr || p->dir != dir || p->doomed) {
+            ReleaseSRWLockExclusive(&g_listings_lock);
+            return false;
+        }
+        if (!p->busy) { p->busy = 1; ReleaseSRWLockExclusive(&g_listings_lock); return true; }
+        ReleaseSRWLockExclusive(&g_listings_lock);
+        SwitchToThread();
+    }
+}
+
+void listing_give(listing* s, bool ended) {
+    AcquireSRWLockExclusive(&g_listings_lock);
+    s->busy = 0;
+    const bool release = ended || s->doomed;
+    if (release) {
+        for (listing** p = &g_listings; *p != nullptr; p = &(*p)->next)
+            if (*p == s) { *p = s->next; break; }
+    }
+    ReleaseSRWLockExclusive(&g_listings_lock);
+    if (release) listing_release(s);
+}
+
 }  // namespace
 
 extern "C" {
@@ -348,7 +418,11 @@ int kal_fs_open(kal_dir base, const char* name, kal_uintptr len,
 
 void kal_fs_close_dir(kal_dir d) {
     void* h = dir_handle(d);
-    if (h) { okw::retire(d.h); okw::NtClose(h); }
+    if (!h) return;
+    // Before the close, because after it the same value may already name
+    // another directory whose enumerations are not this one's.
+    listings_end(h);
+    okw::retire(d.h); okw::NtClose(h);
 }
 
 void kal_fs_close_file(kal_file f) {
@@ -647,16 +721,24 @@ int kal_fs_list_begin(kal_dir d, kal_uintptr* iter) {
 
     auto* s = static_cast<listing*>(kal_alloc(sizeof(listing), alignof(listing)));
     if (!s) { okw::NtClose(own); return kal_err_no_memory; }
-    s->handle = own; s->used = 0; s->at = 0; s->first = true;
+    s->handle = own; s->dir = root; s->busy = 0; s->doomed = 0;
+    s->used = 0; s->at = 0; s->first = true;
+    AcquireSRWLockExclusive(&g_listings_lock);
+    s->next = g_listings; g_listings = s;
+    ReleaseSRWLockExclusive(&g_listings_lock);
     *iter = reinterpret_cast<kal_uintptr>(s);
     return kal_ok;
 }
 
-int kal_fs_list_next(kal_dir, kal_uintptr* iter,
+int kal_fs_list_next(kal_dir d, kal_uintptr* iter,
                      char* name_out, kal_uintptr name_cap,
                      kal_uintptr* name_len, int* kind) {
-    if (iter == nullptr || *iter == 0) return kal_err_invalid;
+    // A closed directory is a released handle (clause 6.7), and closing it
+    // released the iterator too, which fs.h states.
+    void* root = dir_handle(d);
+    if (!root || iter == nullptr || *iter == 0) return kal_err_invalid;
     auto* s = reinterpret_cast<listing*>(*iter);
+    if (!listing_take(s, root)) return kal_err_invalid;
 
     for (;;) {
         if (s->at >= s->used) {
@@ -667,8 +749,7 @@ int kal_fs_list_next(kal_dir, kal_uintptr* iter,
                 okw::file_directory_information_class, 0, nullptr, s->first ? 1 : 0);
             s->first = false;
             if (!okw::ok(r)) {
-                okw::NtClose(s->handle);
-                kal_free(s, sizeof(listing), alignof(listing));
+                listing_give(s, true);
                 *iter = 0;
                 if (name_len) *name_len = 0;
                 // The end of a directory is reported here as a distinct status
@@ -697,6 +778,7 @@ int kal_fs_list_next(kal_dir, kal_uintptr* iter,
         if (kind) *kind = (e->file_attributes & FILE_ATTRIBUTE_REPARSE_POINT) ? kal_node_link
                         : (e->file_attributes & FILE_ATTRIBUTE_DIRECTORY)     ? kal_node_directory
                                                                               : kal_node_file;
+        listing_give(s, false);
         return kal_ok;
     }
 }

src/task.cpp

@@ -28,19 +28,41 @@ DWORD __stdcall run(void* p) {
     return 0;
 }
 
-}  // namespace
+// THE SIZE A CONTEXT IS GIVEN WHEN IT ASKS FOR NONE. This environment would
+// reserve what the program's header states, 1 MiB by default, and the two other
+// implementations give a context the 8 MiB the host's C library on their systems
+// gives a thread; a program that asks for nothing --- C++'s `std::thread' cannot
+// ask --- is given the same room on all three. It is a reservation, committed as
+// the stack grows, so the size costs address space and not memory.
+//
+// No guard is added. This environment places its own below the committed part
+// of the stack and moves it as the stack grows, and the compilers that target it
+// probe a frame larger than a page one page at a time, so a large frame cannot
+// step over it.
+constexpr unsigned long long kDefaultStack = 8ull * 1024u * 1024u;
+
+// What a request for `want' bytes is reserved as. The bounds this environment
+// reports to the context include the pages it keeps for its guard, so the
+// reservation exceeds the request by more than those; what the context is told
+// by kal_task_stack is then at least what it asked for.
+constexpr unsigned long long kSlack = 256u * 1024u;
+
+unsigned long long stack_for(unsigned long long want) {
+    if (want > (1ull << 46)) return 0;
+    const unsigned long long w = (want + 0xffffu) & ~0xffffull;   // the reservation granularity
+    return (w > kDefaultStack ? w : kDefaultStack) + kSlack;
+}
 
-extern "C" {
+constexpr DWORD stack_size_param_is_a_reservation = 0x00010000;
 
-int kal_task_start(void (*entry)(void*), void* arg, kal_task* out) {
+int start(void (*entry)(void*), void* arg, unsigned long long want, kal_task* out) {
     if (entry == nullptr || out == nullptr) return kal_err_invalid;
+    const unsigned long long bytes = stack_for(want);
+    if (bytes == 0) return kal_err_no_memory;
     auto* c = static_cast<context*>(kal_alloc(sizeof(context), alignof(context)));
     if (!c) return kal_err_no_memory;
     c->entry = entry; c->arg = arg;
-    // The stack is the implementation's, which is what the specification says
-    // of it: an environment that does not allocate stacks separately cannot
-    // honour a request for one, so there is no parameter for it.
-    void* h = CreateThread(nullptr, 0, run, c, 0, nullptr);
+    void* h = CreateThread(nullptr, bytes, run, c, stack_size_param_is_a_reservation, nullptr);
     if (!h) {
         const int e = okw::translate_win32(GetLastError());
         kal_free(c, sizeof(context), alignof(context));
@@ -51,6 +73,21 @@ int kal_task_start(void (*entry)(void*), void* arg, kal_task* out) {
     return kal_ok;
 }
 
+}  // namespace
+
+extern "C" {
+
+int kal_task_start(void (*entry)(void*), void* arg, kal_task* out) {
+    return start(entry, arg, 0, out);
+}
+
+// A least size, version 0.16: at least the size asked for, and never less than
+// the default.
+int kal_task_start_sized(void (*entry)(void*), void* arg, kal_uintptr stack_size,
+                         kal_task* out) {
+    return start(entry, arg, stack_size, out);
+}
+
 int kal_task_join(kal_task t) {
     auto* c = reinterpret_cast<context*>(t.h);
     if (!c) return kal_err_invalid;

src/terminal.cpp

@@ -21,6 +21,7 @@ bool  valid(void* h) { return h != nullptr && h != INVALID_HANDLE_VALUE; }
 constexpr DWORD enable_processed_input = 0x0001;
 constexpr DWORD enable_line_input      = 0x0002;
 constexpr DWORD enable_echo_input      = 0x0004;
+constexpr DWORD enable_virtual_terminal_input = 0x0200;
 
 // KAL_TERM_PASS_CONTROL IS ENABLE_PROCESSED_INPUT, INVERTED. This environment
 // keeps the keystrokes it reserves --- the interrupt, and the pair that stops
@@ -50,6 +51,19 @@ int kal_terminal_get_mode(kal_stream s, kal_uintptr* mode) {
     if ((m & enable_line_input) != 0)      out |= KAL_TERM_LINE_EDIT;
     if ((m & enable_echo_input) != 0)      out |= KAL_TERM_ECHO;
     if ((m & enable_processed_input) == 0) out |= KAL_TERM_PASS_CONTROL;
+    // KAL_TERM_PASS_INPUT IS VIRTUAL TERMINAL INPUT WITH LINE INPUT OFF.
+    //
+    // Measured on a console this implementation's tests allocate, with the keys
+    // written into its input as the key events a keyboard produces. With line
+    // input off and virtual terminal input off, the return key arrives as 0x0d
+    // and Ctrl+J arrives as NOTHING: the console consumes the key and delivers
+    // no byte, which is an alteration of what was typed. With virtual terminal
+    // input on, the return key is 0x0d and Ctrl+J is 0x0a --- the two bytes the
+    // other environments deliver with the position set. With line input on,
+    // the return key ends a line as 0x0d 0x0a and no switch changes that, so the
+    // position reads clear there.
+    if ((m & enable_line_input) == 0 && (m & enable_virtual_terminal_input) != 0)
+        out |= KAL_TERM_PASS_INPUT;
     *mode = out;
     return kal_ok;
 }
@@ -65,6 +79,7 @@ int kal_terminal_set_mode(kal_stream s, kal_uintptr mode) {
     // would find its console changed in ways it did not ask for.
     DWORD m = 0;
     if (!GetConsoleMode(h, &m)) return kal_err_not_supported;
+    const bool unaltered = (m & enable_line_input) == 0 && (m & enable_virtual_terminal_input) != 0;
 
     if ((mode & KAL_TERM_LINE_EDIT) != 0) m |=  enable_line_input;
     else                                  m &= ~enable_line_input;
@@ -80,9 +95,29 @@ int kal_terminal_set_mode(kal_stream s, kal_uintptr mode) {
     // several mechanisms; this one stands for exactly one, and assigning it
     // from the mode word is the same act as leaving it alone.
 
+    // KAL_TERM_PASS_INPUT is virtual terminal input, and only while line input
+    // is off: with lines assembled this environment alters input whatever the
+    // switch says, so a request for the position there leaves it as it stands,
+    // which is what terminal.h states of such an environment. A position whose
+    // requested value is the one in effect is not written.
+    bool asked_vt = false;
+    if (((mode & KAL_TERM_PASS_INPUT) != 0) != unaltered) {
+        if ((mode & KAL_TERM_PASS_INPUT) != 0) {
+            if ((mode & KAL_TERM_LINE_EDIT) == 0) { m |= enable_virtual_terminal_input; asked_vt = true; }
+        } else {
+            m &= ~enable_virtual_terminal_input;
+        }
+    }
+
     // A position this implementation does not distinguish is ignored rather than
-    // refused, which clause 6.2 requires of a word.
-    if (!SetConsoleMode(h, m)) return kal_err_not_supported;
+    // refused, which clause 6.2 requires of a word. A console that predates
+    // virtual terminal input refuses the whole word when it is asked for, so
+    // the word is written again without it: the other positions are
+    // established, and the read-back says the fourth was not.
+    if (!SetConsoleMode(h, m)) {
+        if (!asked_vt || !SetConsoleMode(h, m & ~enable_virtual_terminal_input))
+            return kal_err_not_supported;
+    }
     return kal_ok;
 }