Merge branch 'wt/lock' into api-hardening

# Conflicts:
#	src/screen.c
#	tests/term_test.c

10 files changed, 193 insertions, 44 deletions

diff --git a/README.md b/README.md
index c3c5d89..221d56d 100644
--- a/README.md
+++ b/README.md
@@ -2,7 +2,7 @@
 
 term is a headless terminal core: the parser, screen ring, logical-line
 history store, write core, and viewport snapshots. It is a C11 library
-with no dependency but libm.
+with no dependency but libm and, on POSIX, the thread library (`-pthread`).
 
 | Path | Contents |
 | --- | --- |
@@ -23,14 +23,16 @@ Pick one of these:
   sets the lowest instruction set, such as `-march=x86-64-v3`.
 - **Amalgamation**: run `tools/amalgamate.py . OUT`. It writes
   `OUT/term.c` and `OUT/term.h`. Compile `term.c` with any C11 compiler
-  and link libm. A Rust `build.rs` needs one
+  and link libm and the thread library: `cc -std=c11 -pthread -c term.c`
+  on POSIX. A Rust `build.rs` needs one
   `cc::Build::new().file("term.c").compile("term")`.
 - **Sources**: compile `src/*.c` and `src/osc/*.c` once, with `src` and
-  `include/term` on the include path.
+  `include/term` on the include path. Link the thread library too.
 
-The library needs nothing else. The `*-amalgamation` tests run the term
-tests against the generated `term.c`, and `term-cross-test` compiles it
-for aarch64, macOS, and Windows.
+The library needs nothing else. On Windows the writer lock is an SRWLOCK
+from the system library, so it needs no extra link. The `*-amalgamation`
+tests run the term tests against the generated `term.c`, and
+`term-cross-test` compiles it for aarch64, macOS, and Windows.
 
 ### SIMD level
 
diff --git a/include/term/term.h b/include/term/term.h
index 9d5a882..d8758e7 100644
--- a/include/term/term.h
+++ b/include/term/term.h
@@ -17,8 +17,9 @@
  *
  * Threads. Any thread can call any function. Each call that reads or changes
  * a terminal enters its writer section, so calls on one terminal run one at a
- * time. A snapshot belongs to one thread at a time. term_free() is the last
- * call on a terminal.
+ * time. A thread that waits for the section sleeps. On POSIX the section is a
+ * mutex with priority inheritance where the platform has it. A snapshot
+ * belongs to one thread at a time. term_free() is the last call on a terminal.
  */
 
 #include <stdbool.h>
@@ -196,9 +197,10 @@ TERM_API struct term *term_new(const struct term_options *opt);
 TERM_API void term_free(struct term *t);
 
 /* Hold the writer section across several calls, so that no other thread
- * changes the terminal between them. The section is a spin lock: keep it
- * short. The thread that holds it can call every function, and it can enter
- * the section again. */
+ * changes the terminal between them. The section is a mutex, and a waiting
+ * thread sleeps. Keep it short. The thread that holds it can call every
+ * function, and it can enter the section again. Each term_lock() needs one
+ * term_unlock(). */
 TERM_API void term_lock(struct term *t);
 TERM_API void term_unlock(struct term *t);
 
diff --git a/meson.build b/meson.build
index d23c0b4..fa450f9 100644
--- a/meson.build
+++ b/meson.build
@@ -15,6 +15,8 @@ project('term', 'c',
 
 cc = meson.get_compiler('c')
 m_dep = cc.find_library('m', required: false)
+# The writer lock is a pthread mutex on POSIX and an SRWLOCK on Windows.
+thread_dep = dependency('threads')
 if get_option('arch_flag') != ''
   add_project_arguments(get_option('arch_flag'), language: 'c')
 endif
@@ -29,7 +31,7 @@ subdir('src')
 term_dep = declare_dependency(
   include_directories: term_inc,
   link_with: term_lib.get_static_lib(),
-  dependencies: m_dep,
+  dependencies: [m_dep, thread_dep],
 )
 meson.override_dependency('term', term_dep)
 
@@ -37,7 +39,7 @@ meson.override_dependency('term', term_dep)
 term_internal_dep = declare_dependency(
   include_directories: [term_inc, term_private_inc],
   link_with: term_lib.get_static_lib(),
-  dependencies: m_dep,
+  dependencies: [m_dep, thread_dep],
 )
 
 # The short forms of term.h that the term tests use. A host project can use
diff --git a/src/compat.h b/src/compat.h
index cb62679..6c3302f 100644
--- a/src/compat.h
+++ b/src/compat.h
@@ -14,6 +14,19 @@
 #include <stdlib.h>
 #include <string.h>
 
+#if defined(_WIN32)
+#ifndef WIN32_LEAN_AND_MEAN
+#define WIN32_LEAN_AND_MEAN
+#endif
+#ifndef NOMINMAX
+#define NOMINMAX
+#endif
+#include <windows.h>
+#else
+#include <pthread.h>
+#include <unistd.h>
+#endif
+
 #if defined(__GNUC__) || defined(__clang__)
 #define TERM_COMPILER_GNU 1
 #elif defined(_MSC_VER)
@@ -184,6 +197,68 @@ static inline void term_cpu_relax(void)
 #endif
 }
 
+/* The writer lock. POSIX uses a mutex with priority inheritance where the
+ * platform has it, so a waiter lifts the thread that holds the lock. Windows
+ * uses an SRWLOCK. Neither nests, so term_write_begin() counts re-entry. */
+#if defined(_WIN32)
+typedef SRWLOCK term_mutex;
+#else
+typedef pthread_mutex_t term_mutex;
+/* Strict C11 can hide PTHREAD_PRIO_INHERIT in pthread.h, as zig's aarch64
+ * glibc headers do. Every POSIX system that has the option uses 1 for it. */
+#if !defined(PTHREAD_PRIO_INHERIT)
+#define PTHREAD_PRIO_INHERIT 1
+#endif
+#endif
+
+/* Returns false when the lock cannot be made. */
+static inline bool term_mutex_init(term_mutex *m)
+{
+#if defined(_WIN32)
+    InitializeSRWLock(m);
+    return true;
+#elif defined(_POSIX_THREAD_PRIO_INHERIT) && _POSIX_THREAD_PRIO_INHERIT > 0
+    pthread_mutexattr_t attr;
+    if (pthread_mutexattr_init(&attr) != 0)
+        return false;
+    bool ok = pthread_mutexattr_setprotocol(&attr, PTHREAD_PRIO_INHERIT) == 0 &&
+              pthread_mutex_init(m, &attr) == 0;
+    pthread_mutexattr_destroy(&attr);
+    return ok;
+#else
+    return pthread_mutex_init(m, NULL) == 0;
+#endif
+}
+
+/* A normal mutex fails only when it is broken, so the result is ignored. */
+static inline void term_mutex_lock(term_mutex *m)
+{
+#if defined(_WIN32)
+    AcquireSRWLockExclusive(m);
+#else
+    (void)pthread_mutex_lock(m);
+#endif
+}
+
+static inline void term_mutex_unlock(term_mutex *m)
+{
+#if defined(_WIN32)
+    ReleaseSRWLockExclusive(m);
+#else
+    (void)pthread_mutex_unlock(m);
+#endif
+}
+
+/* An SRWLOCK has no destroy. */
+static inline void term_mutex_destroy(term_mutex *m)
+{
+#if defined(_WIN32)
+    (void)m;
+#else
+    (void)pthread_mutex_destroy(m);
+#endif
+}
+
 /* strdup is POSIX, so the core copies strings itself. */
 static inline char *term_strdup(const char *s)
 {
diff --git a/src/internal.h b/src/internal.h
index 5da114d..676413c 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -108,7 +108,8 @@ struct resize_scratch {
 
 
 struct term {
-    _Atomic uint64_t seq;
+    /* The writer section. A waiting thread sleeps on it. */
+    term_mutex write_lock;
     _Atomic uint64_t writer_tid;
     uint32_t write_depth;
     int cols;
@@ -377,18 +378,7 @@ static inline void term_write_begin(struct term *t)
         t->write_depth++;
         return;
     }
-    uint64_t s;
-    for (;;) {
-        s = atomic_load_explicit(&t->seq, memory_order_relaxed);
-        if (s & 1) {
-            term_cpu_relax();
-            continue;
-        }
-        if (atomic_compare_exchange_weak_explicit(&t->seq, &s, s + 1,
-                                                  memory_order_acquire,
-                                                  memory_order_relaxed))
-            break;
-    }
+    term_mutex_lock(&t->write_lock);
     atomic_store_explicit(&t->writer_tid, self, memory_order_relaxed);
     t->write_depth = 1;
 }
@@ -398,7 +388,7 @@ static inline void term_write_end(struct term *t)
     if (!t) return;
     if (--t->write_depth == 0) {
         atomic_store_explicit(&t->writer_tid, 0, memory_order_relaxed);
-        atomic_fetch_add_explicit(&t->seq, 1, memory_order_release);
+        term_mutex_unlock(&t->write_lock);
     }
 }
 
diff --git a/src/meson.build b/src/meson.build
index 7223caa..7684275 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -25,7 +25,7 @@ term_lib = both_libraries('term',
   'text_run.c',
   term_osc_src,
   include_directories: term_private_inc,
-  dependencies: [m_dep],
+  dependencies: [m_dep, thread_dep],
   gnu_symbol_visibility: 'hidden',
   version: meson.project_version(),
   soversion: term_soversion,
diff --git a/src/screen.c b/src/screen.c
index 0857f17..b99948d 100644
--- a/src/screen.c
+++ b/src/screen.c
@@ -874,8 +874,11 @@ struct term *term_new(const struct term_options *opt)
     struct term *t = calloc(1, sizeof(*t));
     if (!t)
         return NULL;
+    if (!term_mutex_init(&t->write_lock)) {
+        free(t);
+        return NULL;
+    }
     memcpy(t->name, name, strlen(name) + 1);
-    atomic_init(&t->seq, 0);
     atomic_init(&t->writer_tid, 0);
     t->write_depth = 0;
     t->serial = atomic_fetch_add_explicit(&next_serial, 1, memory_order_relaxed);
@@ -890,11 +893,13 @@ struct term *term_new(const struct term_options *opt)
 
     t->next_row_uid = 1;
     if (!screen_setup(&t->scr, cols, rows, &t->next_row_uid)) {
+        term_mutex_destroy(&t->write_lock);
         free(t);
         return NULL;
     }
     if (!term_hist_init(&t->hist, t->sb_capacity, 0)) {
         screen_free(&t->scr);
+        term_mutex_destroy(&t->write_lock);
         free(t);
         return NULL;
     }
@@ -951,6 +956,7 @@ void term_free(struct term *t)
     free(t->tabs);
     term_osc_cleanup(t);
     term_gstore_free(t->gstore);
+    term_mutex_destroy(&t->write_lock);
     free(t);
 }
 
diff --git a/tests/meson.build b/tests/meson.build
index 629f6d5..4fda075 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -77,7 +77,7 @@ term_amalgamation = custom_target(
 term_amalgamation_lib = static_library(
   'term-amalgamation',
   term_amalgamation[0],
-  dependencies: [m_dep],
+  dependencies: [m_dep, thread_dep],
   override_options: ['c_std=c11', 'warning_level=3', 'werror=true'],
   build_by_default: false,
 )
@@ -90,7 +90,7 @@ foreach entry : term_tests
         c_args: test_c_args,
         include_directories: [term_inc, test_helpers_inc],
         link_with: term_amalgamation_lib,
-        dependencies: [m_dep] + entry[2],
+        dependencies: [m_dep, thread_dep] + entry[2],
         build_by_default: false),
       env: test_env, suite: ['unit', 'amalgamation'], timeout: 180)
   endif
diff --git a/tests/term_test.c b/tests/term_test.c
index 51544ef..62b827f 100644
--- a/tests/term_test.c
+++ b/tests/term_test.c
@@ -4,6 +4,7 @@
 #include <assert.h>
 #include <limits.h>
 #include <pthread.h>
+#include <sched.h>
 #include <stdatomic.h>
 #include <stdbool.h>
 #include <stdio.h>
@@ -1837,21 +1838,22 @@ struct concurrent_worker_args {
     _Atomic bool done;
 };
 
+static const char concurrent_stream[] =
+    "The quick brown fox jumps over the lazy dog.\r\n"
+    "\033[31mRed text\033[0m and \033[32mgreen text\033[0m.\r\n"
+    "Some unicode: 漢字, ひらがな, and emojis: 😀🚀🎉\r\n"
+    "\033[2J\033[HCleared and reset\r\n"
+    "Line 1\r\nLine 2\r\nLine 3\r\nLine 4\r\nLine 5\r\n"
+    "\033]7;file:///tmp/dir\007\033]22;pointer\007"
+    "\033]8;;https://example.com\007link\033]8;;\007\r\n";
+
 static void *concurrent_feed_worker(void *arg)
 {
     struct concurrent_worker_args *w = arg;
-    static const char stream[] =
-        "The quick brown fox jumps over the lazy dog.\r\n"
-        "\033[31mRed text\033[0m and \033[32mgreen text\033[0m.\r\n"
-        "Some unicode: 漢字, ひらがな, and emojis: 😀🚀🎉\r\n"
-        "\033[2J\033[HCleared and reset\r\n"
-        "Line 1\r\nLine 2\r\nLine 3\r\nLine 4\r\nLine 5\r\n"
-        "\033]7;file:///tmp/dir\007\033]22;pointer\007"
-        "\033]8;;https://example.com\007link\033]8;;\007\r\n";
-    size_t len = strlen(stream);
+    size_t len = strlen(concurrent_stream);
 
     for (int i = 0; i < 2000; i++) {
-        term_feed(w->t, stream, len, 0);
+        term_feed(w->t, concurrent_stream, len, 0);
     }
     atomic_store(&w->done, true);
     return NULL;
@@ -1859,10 +1861,11 @@ static void *concurrent_feed_worker(void *arg)
 
 static void test_concurrent_feed_and_snapshot(void)
 {
-    struct term *t = term_new(&(struct term_options){
+    struct term_options opt = {
         .size = { .x = 80, .y = 24 },
         .scrollback = 1000,
-    });
+    };
+    struct term *t = term_new(&opt);
     assert(t != NULL);
 
     struct concurrent_worker_args w = { .t = t, .done = false };
@@ -1903,6 +1906,33 @@ static void test_concurrent_feed_and_snapshot(void)
     bool ok = term_snapshot_update(snap, t);
     assert(ok);
 
+    /* The reader changed only the view and the selection. The screen and
+     * the history must match a terminal that got the same bytes on one
+     * thread. */
+    struct term *ref = term_new(&opt);
+    assert(ref != NULL);
+    for (int i = 0; i < 2000; i++)
+        tt_write(ref, concurrent_stream, strlen(concurrent_stream));
+    assert(tt_cols(t) == tt_cols(ref) && tt_rows(t) == tt_rows(ref));
+    assert(tt_history(t) == tt_history(ref));
+    assert(tt_cursor_x(t) == tt_cursor_x(ref) && tt_cursor_y(t) == tt_cursor_y(ref));
+    char *got = malloc((size_t)tt_cols(t) + 1);
+    char *want = malloc((size_t)tt_cols(ref) + 1);
+    assert(got && want);
+    for (int y = 0; y < tt_rows(t); y++) {
+        row_text(t, y, got);
+        row_text(ref, y, want);
+        assert(!strcmp(got, want));
+    }
+    free(got);
+    free(want);
+    char *text = term_history_text(t, 1000);
+    char *text_ref = term_history_text(ref, 1000);
+    assert(text && text_ref && !strcmp(text, text_ref));
+    term_text_free(text);
+    term_text_free(text_ref);
+
+    term_free(ref);
     term_snapshot_free(snap);
     term_free(t);
 }
@@ -1928,10 +1958,52 @@ static void test_reserved_options_must_be_zero(void)
     term_free(t);
 }
 
+struct lock_probe {
+    struct term *t;
+    _Atomic bool entered;
+};
+
+static void *lock_probe_worker(void *arg)
+{
+    struct lock_probe *p = arg;
+    term_lock(p->t);
+    atomic_store(&p->entered, true);
+    term_unlock(p->t);
+    return NULL;
+}
+
+/* term_lock() nests on the owning thread. Another thread waits until the
+ * last term_unlock(). */
+static void test_lock_reentrant_across_threads(void)
+{
+    struct term *t = term_new(&(struct term_options){
+        .size = { .x = 80, .y = 24 },
+    });
+    assert(t != NULL);
+
+    term_lock(t);
+    term_lock(t);
+    assert(tt_cols(t) == 80);
+    term_unlock(t);
+
+    struct lock_probe p = { .t = t, .entered = false };
+    pthread_t th;
+    assert(pthread_create(&th, NULL, lock_probe_worker, &p) == 0);
+    for (int i = 0; i < 1000; i++)
+        sched_yield();
+    assert(!atomic_load(&p.entered));
+    term_unlock(t);
+
+    assert(pthread_join(th, NULL) == 0);
+    assert(atomic_load(&p.entered));
+    term_free(t);
+}
+
 int main(void)
 {
     test_reserved_options_must_be_zero();
     test_concurrent_feed_and_snapshot();
+    test_lock_reentrant_across_threads();
     test_grow_past_reservation();
     test_skipped_cells_read_blank();
     test_csi_text_chunks();
diff --git a/tools/amalgamate.py b/tools/amalgamate.py
index f2cdf3b..09c6c91 100644
--- a/tools/amalgamate.py
+++ b/tools/amalgamate.py
@@ -3,7 +3,7 @@
 
 term.h joins the public headers. term.c joins every source file and puts
 each private header in at its first include. A host compiles term.c once,
-with any C11 compiler, and links libm.
+with any C11 compiler, and links libm and the thread library.
 
     amalgamate.py TERM_DIR OUT_DIR [DEPFILE]