From e3d8fe4854a7ef7ff0637688cda9e65feb680122 Mon Sep 17 00:00:00 2001 From: Berke Güzel Date: Thu, 15 Oct 2026 12:37:10 +0300 Subject: Replace the writer spin lock with a blocking mutex term_write_begin spun on an atomic sequence word. A waiter burned CPU, and with no priority inheritance a SCHED_FIFO waiter could starve a preempted holder on the same CPU. The writer section is now a pthread mutex with PTHREAD_PRIO_INHERIT where the platform has it, and an SRWLOCK on Windows. Re-entry on the owning thread still only counts the depth. The platform wrapper (init, lock, unlock, destroy) is in compat.h. term_new initializes the mutex, and term_free destroys it. Meson links the thread library into the core, term_dep, and term_internal_dep. The README, term.h, and the amalgamation note say that POSIX needs -pthread. Tests: the feed and snapshot thread test now checks the final screen and history against a serial reference. A new test checks that term_lock nests and that another thread waits for the last term_unlock. Co-Authored-By: Claude Haiku 5.5 --- src/compat.h | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/internal.h | 18 +++----------- src/meson.build | 2 +- src/screen.c | 8 +++++- 4 files changed, 87 insertions(+), 16 deletions(-) (limited to 'src') 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 #include +#if defined(_WIN32) +#ifndef WIN32_LEAN_AND_MEAN +#define WIN32_LEAN_AND_MEAN +#endif +#ifndef NOMINMAX +#define NOMINMAX +#endif +#include +#else +#include +#include +#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 8c02c91..d67e67e 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; @@ -375,18 +376,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; } @@ -396,7 +386,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 694ee0e..6ce27ce 100644 --- a/src/meson.build +++ b/src/meson.build @@ -21,7 +21,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', install: true ) diff --git a/src/screen.c b/src/screen.c index 87d6f2c..82cd6d3 100644 --- a/src/screen.c +++ b/src/screen.c @@ -856,7 +856,10 @@ struct term *term_new(const struct term_options *opt) struct term *t = calloc(1, sizeof(*t)); if (!t) return NULL; - atomic_init(&t->seq, 0); + if (!term_mutex_init(&t->write_lock)) { + free(t); + return NULL; + } atomic_init(&t->writer_tid, 0); t->write_depth = 0; t->serial = atomic_fetch_add_explicit(&next_serial, 1, memory_order_relaxed); @@ -871,11 +874,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; } @@ -932,6 +937,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); } -- cgit v1.2.3