From 6bdc8dd121a8caf542103e3dd59a6638093ac5c0 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:44:52 +0800 Subject: [PATCH 01/17] Never claim the uninitialized idle job A failed compare_exchange writes the current value back into the expected operand, so the reloaded pointer can be the idle sentinel once another worker drains the queue. That sentinel is allocated with plain malloc and its func and future fields are never initialized, so claiming it dereferences indeterminate pointers. Under a sanitizer the previous code aborts in roughly 2 runs out of 30 with a heap-use-after-free. Rule the sentinel out on every iteration rather than once before the loop, and update the two listings in the manuscript that quote it. The versioned variant carries the same defect and is repaired separately, because there it cannot be separated from how the queue head is published. --- concurrency-primer.tex | 16 +++++++++++----- examples/rmw_example.c | 12 ++++++++---- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/concurrency-primer.tex b/concurrency-primer.tex index 2f85c48..6a2f7b9 100644 --- a/concurrency-primer.tex +++ b/concurrency-primer.tex @@ -564,11 +564,16 @@ \subsection{Example} Otherwise, the last job is under race. The while loop in the function \monobox{worker}, \begin{ccode} -while (!atomic_compare_exchange_weak(&thrd_pool->head->prev, &job, - job->prev)) { +while (job != &thrd_pool->head->job && + !atomic_compare_exchange_weak(&thrd_pool->head->prev, &job, + job->prev)) { } \end{ccode} -, keeps trying to claim the job atomically until success. +, keeps trying to claim the job atomically until it succeeds. +A failed compare-exchange writes the current value back into \cc|job|, +so the guard is re-evaluated on every iteration: +another worker may have taken the last job in the meantime, +leaving the idle job, which must never be claimed. Built-in post increment and decrement operators and compound assignment on atomic objects, such as \monobox{++} and \monobox{+=}, are read-modify-write atomic operations with total sequentially consistent ordering as well. They behave equivalently to a \cc|do while| loop. See \clang{}11 standard 6.5.2.4 and 6.5.16.2 for more details. @@ -939,8 +944,9 @@ \subsection{ABA problem} \begin{ccode} job_t *job = atomic_load(&thrd_pool->head->prev); ... - while (!atomic_compare_exchange_weak(&thrd_pool->head->prev, &job, - job->prev)) + while (job != &thrd_pool->head->job && + !atomic_compare_exchange_weak(&thrd_pool->head->prev, &job, + job->prev)) ; \end{ccode} diff --git a/examples/rmw_example.c b/examples/rmw_example.c index d5b0c68..e2300f3 100644 --- a/examples/rmw_example.c +++ b/examples/rmw_example.c @@ -77,6 +77,13 @@ static int worker(void *args) if (atomic_load(&thrd_pool->state) == running) { /* worker takes the job */ job_t *job = atomic_load(&thrd_pool->head->prev); + /* A failed compare-exchange reloads "job", so the idle job has to + * be ruled out on every iteration, not just once up front. + */ + while (job != &thrd_pool->head->job && + !atomic_compare_exchange_weak(&thrd_pool->head->prev, &job, + job->prev)) + ; /* worker checks if there is only an idle job in the job queue */ if (job == &thrd_pool->head->job) { /* worker says it is idle */ @@ -84,9 +91,6 @@ static int worker(void *args) thrd_yield(); continue; } - while (!atomic_compare_exchange_weak(&thrd_pool->head->prev, &job, - job->prev)) - ; job->future->result = (void *)job->func(job->future->arg); atomic_flag_clear(&job->future->flag); free(job); @@ -94,7 +98,7 @@ static int worker(void *args) /* worker is idle */ thrd_yield(); } - }; + } return EXIT_SUCCESS; } From 499de68427efc7e7b2c2bf26fafea4b2be43187d Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:45:07 +0800 Subject: [PATCH 02/17] Claim and publish the versioned queue head consistently Two defects that cannot be separated in this file. The worker had the same sentinel bug as its unversioned twin: a failed compare-exchange reloads the expected operand, so the reloaded pointer can be the idle job, whose func and future are never initialized. And add_job wrote the pointer and its version as separate stores, even though the union exists so the pair can be swung with one double-width compare-and-swap. A worker compare-exchanging between the two observes the new job against the stale version, which is the state the version number exists to make unrepresentable, and the increment was itself a plain read-modify-write on a location workers update concurrently. Fixing either alone makes the example worse rather than better, which is why they land together: the new claim loop against the old split publication hangs in roughly 2 runs out of 40, and the old claim loop against the new atomic publication fails outright. Together they are clean over the same sample. The store remains unsynchronized, relying on the employer only adding jobs while the pool is idle, so the comment says that rather than claiming nothing races it. --- examples/rmw_example_aba.c | 49 ++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/examples/rmw_example_aba.c b/examples/rmw_example_aba.c index 5449bf2..5eebf3a 100644 --- a/examples/rmw_example_aba.c +++ b/examples/rmw_example_aba.c @@ -86,6 +86,17 @@ static int worker(void *args) if (atomic_load(&thrd_pool->state) == running) { /* worker takes the job */ struct versioned_prev job = atomic_load(&thrd_pool->head->v_prev); + /* A failed compare-exchange reloads "job", so the idle job has to + * be ruled out on every iteration, not just once up front. + */ + while (job.ptr != &thrd_pool->head->job) { + /* compare 16 byte at once */ + struct versioned_prev next = { .ptr = job.ptr->prev, + ._version = job._version }; + if (atomic_compare_exchange_weak(&thrd_pool->head->v_prev, &job, + next)) + break; + } /* worker checks if there is only an idle job in the job queue */ if (job.ptr == &thrd_pool->head->job) { /* worker says it is idle */ @@ -94,14 +105,6 @@ static int worker(void *args) continue; } - struct versioned_prev next; - /* compare 16 byte at once */ - do { - next.ptr = job.ptr->prev; - next._version = job._version; - } while (!atomic_compare_exchange_weak(&thrd_pool->head->v_prev, - &job, next)); - job.ptr->future->result = (void *)job.ptr->func(job.ptr->future->arg); atomic_flag_clear(&job.ptr->future->flag); @@ -110,7 +113,7 @@ static int worker(void *args) /* worker is idle */ thrd_yield(); } - }; + } return EXIT_SUCCESS; } @@ -203,9 +206,31 @@ struct tpool_future *add_job(tpool_t *thrd_pool, void *(*func)(void *), job->prev = &thrd_pool->head->job; thrd_pool->head->job.next->prev = job; thrd_pool->head->job.next = job; - if (thrd_pool->head->prev == &thrd_pool->head->job) { - thrd_pool->head->prev = job; - thrd_pool->head->version += 1; + + struct versioned_prev cur = atomic_load(&thrd_pool->head->v_prev); + bool was_empty = cur.ptr == &thrd_pool->head->job; + if (was_empty) + thrd_pool->head->job.next = &thrd_pool->head->job; + + job->func = func; + job->future = future; + job->next = thrd_pool->head->job.next; + job->prev = &thrd_pool->head->job; + thrd_pool->head->job.next->prev = job; + thrd_pool->head->job.next = job; + + if (was_empty) { + /* Publish the pointer and its version in one 16-byte store. Writing + * the two halves separately would let a worker observe the new job + * paired with the old version, which is precisely the window the + * version number exists to close. This store is unsynchronized: it + * relies on the employer only adding jobs while the pool is idle, + * which a worker preempted just after its own state check does not + * honor. That window is the same one the ABA discussion describes. + */ + struct versioned_prev next = { .ptr = job, + ._version = cur._version + 1 }; + atomic_store(&thrd_pool->head->v_prev, next); /* the previous job of the idle job is itself */ thrd_pool->head->job.prev = &thrd_pool->head->job; } From 6b75673912e077600815769850000969adb59f33 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:45:21 +0800 Subject: [PATCH 03/17] Wait for the workers to park before destroying the pool tpool_destroy reports on a pool it cancels while it is still running, and main reached it as soon as the last future completed, which does not mean the last worker has published idle. The window is narrow enough that the pool examples do not print the diagnostic today, but it widens as soon as the claim loop is touched, and later commits gate CI on this output matching what the manuscript quotes. wait_until is already there for the same purpose earlier in main. --- examples/rmw_example.c | 7 ++++++- examples/rmw_example_aba.c | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/examples/rmw_example.c b/examples/rmw_example.c index e2300f3..6e898b8 100644 --- a/examples/rmw_example.c +++ b/examples/rmw_example.c @@ -237,7 +237,12 @@ int main() tpool_future_destroy(futures[i]); } - /* employer destroys the job queue and lays workers off */ + /* employer destroys the job queue and lays workers off. Wait for the + * workers to park first: tpool_destroy reports on a pool it cancels + * while running, and a completed future does not by itself mean the + * last worker has published idle. + */ + wait_until(&thrd_pool, idle); tpool_destroy(&thrd_pool); printf("PI calculated with %d terms: %.15f\n", PRECISION, bbp_sum); return 0; diff --git a/examples/rmw_example_aba.c b/examples/rmw_example_aba.c index 5eebf3a..9ddca73 100644 --- a/examples/rmw_example_aba.c +++ b/examples/rmw_example_aba.c @@ -276,7 +276,12 @@ int main() tpool_future_destroy(futures[i]); } - /* employer destroys the job queue and lays workers off */ + /* employer destroys the job queue and lays workers off. Wait for the + * workers to park first: tpool_destroy reports on a pool it cancels + * while running, and a completed future does not by itself mean the + * last worker has published idle. + */ + wait_until(&thrd_pool, idle); tpool_destroy(&thrd_pool); printf("PI calculated with %d terms: %.15f\n", PRECISION, bbp_sum); return 0; From 8728524295c322117bf51ea55846e037421c5b1c Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:45:35 +0800 Subject: [PATCH 04/17] Drop the stale front link when the queue drains Workers pop from the back and free as they go, but nothing ever updates head->job.next, so once the queue has drained it still names the last job freed and the next add_job writes through that pointer. It cannot fire while main adds a single batch, but the ABA scenario in the manuscript narrates precisely a second batch, so the text describes a flow the code cannot survive. A two-batch driver under a sanitizer aborts in 20 runs out of 20 inside add_job before this change and none after; what remains is the documented use-after-free in worker, which the book keeps deliberately. --- examples/rmw_example.c | 11 ++++++++++- examples/rmw_example_aba.c | 12 +++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/examples/rmw_example.c b/examples/rmw_example.c index 6e898b8..a8fb80a 100644 --- a/examples/rmw_example.c +++ b/examples/rmw_example.c @@ -184,13 +184,22 @@ struct tpool_future *add_job(tpool_t *thrd_pool, void *(*func)(void *), return NULL; } + /* Workers pop from the back and free as they go, but nothing updates the + * front link on the way, so once the queue has drained head->job.next + * still names the last job freed. Drop it before linking, otherwise the + * writes below land in freed memory. + */ + bool was_empty = thrd_pool->head->prev == &thrd_pool->head->job; + if (was_empty) + thrd_pool->head->job.next = &thrd_pool->head->job; + job->func = func; job->future = future; job->next = thrd_pool->head->job.next; job->prev = &thrd_pool->head->job; thrd_pool->head->job.next->prev = job; thrd_pool->head->job.next = job; - if (thrd_pool->head->prev == &thrd_pool->head->job) { + if (was_empty) { thrd_pool->head->prev = job; /* the previous job of the idle job is itself */ thrd_pool->head->job.prev = &thrd_pool->head->job; diff --git a/examples/rmw_example_aba.c b/examples/rmw_example_aba.c index 9ddca73..d3c21b8 100644 --- a/examples/rmw_example_aba.c +++ b/examples/rmw_example_aba.c @@ -200,13 +200,11 @@ struct tpool_future *add_job(tpool_t *thrd_pool, void *(*func)(void *), return NULL; } - job->func = func; - job->future = future; - job->next = thrd_pool->head->job.next; - job->prev = &thrd_pool->head->job; - thrd_pool->head->job.next->prev = job; - thrd_pool->head->job.next = job; - + /* Workers pop from the back and free as they go, but nothing updates the + * front link on the way, so once the queue has drained head->job.next + * still names the last job freed. Drop it before linking, otherwise the + * writes below land in freed memory. + */ struct versioned_prev cur = atomic_load(&thrd_pool->head->v_prev); bool was_empty = cur.ptr == &thrd_pool->head->job; if (was_empty) From c969005de2cef976c0882ea43c8b7bc5180b2aa4 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:45:49 +0800 Subject: [PATCH 05/17] Align the idle job to a cache line The struct carries a cache line of padding and a comment saying it avoids false sharing, but it was allocated with plain malloc, so it landed on a 16-byte boundary and the hot pointer shared a line with whatever preceded it. Padding sizes a struct; only alignment places it. In the versioned variant the same alignment is what the double-width compare-and-swap needs. Declaring it makes sizeof a multiple of the alignment, which is what aligned_alloc requires of its size argument. --- examples/rmw_example.c | 12 ++++++++++-- examples/rmw_example_aba.c | 14 ++++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/examples/rmw_example.c b/examples/rmw_example.c index a8fb80a..59f293a 100644 --- a/examples/rmw_example.c +++ b/examples/rmw_example.c @@ -23,7 +23,11 @@ typedef struct job { } job_t; typedef struct idle_job { - _Atomic(job_t *) prev; + /* Padding alone only sizes the struct; the alignment is what keeps "prev" + * off a cache line shared with anything else, and it requires an + * allocation aligned to match. See tpool_init. + */ + _Alignas(CACHE_LINE_SIZE) _Atomic(job_t *) prev; char padding[CACHE_LINE_SIZE - sizeof(_Atomic(job_t *))]; /* avoid false sharing */ job_t job; @@ -116,7 +120,11 @@ static bool tpool_init(tpool_t *thrd_pool, size_t size) return false; } - idle_job_t *idle_job = malloc(sizeof(idle_job_t)); + /* aligned_alloc, not malloc: the cache line padding in idle_job_t is only + * worth anything if the allocation starts on a cache line boundary. + */ + idle_job_t *idle_job = + aligned_alloc(_Alignof(idle_job_t), sizeof(idle_job_t)); if (!idle_job) { printf("Failed to allocate idle job.\n"); return false; diff --git a/examples/rmw_example_aba.c b/examples/rmw_example_aba.c index d3c21b8..3273f17 100644 --- a/examples/rmw_example_aba.c +++ b/examples/rmw_example_aba.c @@ -23,7 +23,12 @@ typedef struct job { } job_t; typedef struct idle_job { - union { + /* Padding alone only sizes the struct; the alignment is what keeps the + * union off a cache line shared with anything else, and it is also what + * guarantees the 16-byte alignment a double-width CAS requires. It needs + * an allocation aligned to match: see tpool_init. + */ + _Alignas(CACHE_LINE_SIZE) union { struct { _Atomic(job_t *) prev; unsigned long long version; @@ -131,7 +136,12 @@ static bool tpool_init(tpool_t *thrd_pool, size_t size) return false; } - idle_job_t *idle_job = malloc(sizeof(idle_job_t)); + /* aligned_alloc, not malloc: the double-width CAS on v_prev needs the + * union 16-byte aligned, and the cache line padding is only worth + * anything if the allocation starts on a cache line boundary. + */ + idle_job_t *idle_job = + aligned_alloc(_Alignof(idle_job_t), sizeof(idle_job_t)); if (!idle_job) { printf("Failed to allocate idle job.\n"); return false; From ece0179455f7d1339db2046b5da71598373d3b18 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:46:02 +0800 Subject: [PATCH 06/17] Clean up after a failed thread pool init thrd_create was unchecked, so a failure left an indeterminate thread identifier that tpool_destroy would later join. The allocation failure paths leaked the identifier array and, more awkwardly, returned false with the initialized flag still set, which wedged the pool object against any retry. Check every create, join the workers already hired, release the flag, and leave no dangling pointer behind. Nothing is left for tpool_destroy to reclaim on this path, so the comment says the caller must not call it rather than inviting them to. --- examples/rmw_example.c | 28 ++++++++++++++++++++++++++-- examples/rmw_example_aba.c | 28 ++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/examples/rmw_example.c b/examples/rmw_example.c index 59f293a..9fb2aea 100644 --- a/examples/rmw_example.c +++ b/examples/rmw_example.c @@ -117,6 +117,8 @@ static bool tpool_init(tpool_t *thrd_pool, size_t size) thrd_pool->pool = malloc(sizeof(thrd_t) * size); if (!thrd_pool->pool) { printf("Failed to allocate thread identifiers.\n"); + /* release the claim, otherwise the pool can never be initialized */ + atomic_flag_clear(&thrd_pool->initialized); return false; } @@ -127,6 +129,8 @@ static bool tpool_init(tpool_t *thrd_pool, size_t size) aligned_alloc(_Alignof(idle_job_t), sizeof(idle_job_t)); if (!idle_job) { printf("Failed to allocate idle job.\n"); + free(thrd_pool->pool); + atomic_flag_clear(&thrd_pool->initialized); return false; } @@ -140,8 +144,28 @@ static bool tpool_init(tpool_t *thrd_pool, size_t size) thrd_pool->size = size; /* employer hires many workers */ - for (size_t i = 0; i < size; i++) - thrd_create(thrd_pool->pool + i, worker, thrd_pool); + for (size_t i = 0; i < size; i++) { + if (thrd_create(thrd_pool->pool + i, worker, thrd_pool) != + thrd_success) { + printf("Failed to create worker %zu.\n", i); + /* lay off whoever was already hired before giving up */ + atomic_store(&thrd_pool->state, cancelled); + while (i--) + thrd_join(thrd_pool->pool[i], NULL); + free(idle_job); + free(thrd_pool->pool); + /* init undoes itself completely, so there is nothing left for + * tpool_destroy to reclaim and the caller must not call it. + * Clear the fields anyway, so a later tpool_init has no stale + * pointer or count to trip over. + */ + thrd_pool->pool = NULL; + thrd_pool->head = NULL; + thrd_pool->size = 0; + atomic_flag_clear(&thrd_pool->initialized); + return false; + } + } return true; } diff --git a/examples/rmw_example_aba.c b/examples/rmw_example_aba.c index 3273f17..708f2dd 100644 --- a/examples/rmw_example_aba.c +++ b/examples/rmw_example_aba.c @@ -133,6 +133,8 @@ static bool tpool_init(tpool_t *thrd_pool, size_t size) thrd_pool->pool = malloc(sizeof(thrd_t) * size); if (!thrd_pool->pool) { printf("Failed to allocate thread identifiers.\n"); + /* release the claim, otherwise the pool can never be initialized */ + atomic_flag_clear(&thrd_pool->initialized); return false; } @@ -144,6 +146,8 @@ static bool tpool_init(tpool_t *thrd_pool, size_t size) aligned_alloc(_Alignof(idle_job_t), sizeof(idle_job_t)); if (!idle_job) { printf("Failed to allocate idle job.\n"); + free(thrd_pool->pool); + atomic_flag_clear(&thrd_pool->initialized); return false; } @@ -158,8 +162,28 @@ static bool tpool_init(tpool_t *thrd_pool, size_t size) thrd_pool->size = size; /* employer hires many workers */ - for (size_t i = 0; i < size; i++) - thrd_create(thrd_pool->pool + i, worker, thrd_pool); + for (size_t i = 0; i < size; i++) { + if (thrd_create(thrd_pool->pool + i, worker, thrd_pool) != + thrd_success) { + printf("Failed to create worker %zu.\n", i); + /* lay off whoever was already hired before giving up */ + atomic_store(&thrd_pool->state, cancelled); + while (i--) + thrd_join(thrd_pool->pool[i], NULL); + free(idle_job); + free(thrd_pool->pool); + /* init undoes itself completely, so there is nothing left for + * tpool_destroy to reclaim and the caller must not call it. + * Clear the fields anyway, so a later tpool_init has no stale + * pointer or count to trip over. + */ + thrd_pool->pool = NULL; + thrd_pool->head = NULL; + thrd_pool->size = 0; + atomic_flag_clear(&thrd_pool->initialized); + return false; + } + } return true; } From 6c9b0c3001e5415776c08bac6a048149c2d179ff Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:46:16 +0800 Subject: [PATCH 07/17] Free unclaimed futures when tearing the pool down Cancelling a pool with jobs still queued freed each job but neither the future it owns nor that future's result. Workers are all joined by this point, so the queue is single-threaded here and can be drained safely. The cancellation warning also fired for any non-zero prior state rather than for a running pool specifically, which is what the manuscript says it reports. --- examples/rmw_example.c | 6 +++++- examples/rmw_example_aba.c | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/examples/rmw_example.c b/examples/rmw_example.c index 9fb2aea..b5c3706 100644 --- a/examples/rmw_example.c +++ b/examples/rmw_example.c @@ -172,14 +172,18 @@ static bool tpool_init(tpool_t *thrd_pool, size_t size) static void tpool_destroy(tpool_t *thrd_pool) { - if (atomic_exchange(&thrd_pool->state, cancelled)) + if (atomic_exchange(&thrd_pool->state, cancelled) == running) printf("Thread pool cancelled with jobs still running.\n"); for (int i = 0; i < thrd_pool->size; i++) thrd_join(thrd_pool->pool[i], NULL); + /* Workers are all joined, so the queue is ours alone now. Unclaimed jobs + * own a future that nobody will ever wait on; free both. + */ while (thrd_pool->head->prev != &thrd_pool->head->job) { job_t *job = thrd_pool->head->prev->prev; + tpool_future_destroy(thrd_pool->head->prev->future); free(thrd_pool->head->prev); thrd_pool->head->prev = job; } diff --git a/examples/rmw_example_aba.c b/examples/rmw_example_aba.c index 708f2dd..398018a 100644 --- a/examples/rmw_example_aba.c +++ b/examples/rmw_example_aba.c @@ -190,14 +190,18 @@ static bool tpool_init(tpool_t *thrd_pool, size_t size) static void tpool_destroy(tpool_t *thrd_pool) { - if (atomic_exchange(&thrd_pool->state, cancelled)) + if (atomic_exchange(&thrd_pool->state, cancelled) == running) printf("Thread pool cancelled with jobs still running.\n"); for (int i = 0; i < thrd_pool->size; i++) thrd_join(thrd_pool->pool[i], NULL); + /* Workers are all joined, so the queue is ours alone now. Unclaimed jobs + * own a future that nobody will ever wait on; free both. + */ while (thrd_pool->head->prev != &thrd_pool->head->job) { job_t *job = thrd_pool->head->prev->prev; + tpool_future_destroy(thrd_pool->head->prev->future); free(thrd_pool->head->prev); thrd_pool->head->prev = job; } From 11c8f184cc87ec0623baa71c40b25700720a37a2 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:46:30 +0800 Subject: [PATCH 08/17] Report job failures through the exit status add_job returns NULL when it cannot allocate, and bbp returns NULL when it cannot allocate its result; both were then dereferenced unchecked. Guarding them without also failing would be worse than the crash, because the program prints a value the manuscript quotes exactly, and a run missing a term would otherwise report success while contradicting the book. A failed init returns non-zero too, having previously returned 0 from a run that did no work. Bailing out mid-loop has to drain the queue first: tpool_destroy frees the futures of jobs still queued, but a worker that has already claimed one frees the job and not its future. Wait for the workers to park before destroying either way, since tpool_destroy reports on a pool it cancels while running and a completed future does not mean the last worker has published idle. --- examples/rmw_example.c | 31 +++++++++++++++++++++++++++---- examples/rmw_example_aba.c | 31 +++++++++++++++++++++++++++---- 2 files changed, 54 insertions(+), 8 deletions(-) diff --git a/examples/rmw_example.c b/examples/rmw_example.c index b5c3706..024e12a 100644 --- a/examples/rmw_example.c +++ b/examples/rmw_example.c @@ -249,7 +249,7 @@ static inline void wait_until(tpool_t *thrd_pool, int state) thrd_yield(); } -int main() +int main(void) { int bbp_args[PRECISION]; struct tpool_future *futures[PRECISION]; @@ -258,7 +258,7 @@ int main() tpool_t thrd_pool = { .initialized = ATOMIC_FLAG_INIT }; if (!tpool_init(&thrd_pool, N_THREADS)) { printf("failed to init.\n"); - return 0; + return EXIT_FAILURE; } /* employer asks workers to work */ atomic_store(&thrd_pool.state, running); @@ -270,15 +270,38 @@ int main() for (int i = 0; i < PRECISION; i++) { bbp_args[i] = i; futures[i] = add_job(&thrd_pool, bbp, &bbp_args[i]); + if (!futures[i]) { + printf("Failed to add job %d.\n", i); + /* Jobs handed out before this point own a future each, and a + * worker that claims one frees the job but not the future. Let + * them drain so those futures can be reclaimed here. + */ + atomic_store(&thrd_pool.state, running); + for (int j = 0; j < i; j++) { + tpool_future_wait(futures[j]); + tpool_future_destroy(futures[j]); + } + /* the pool is drained, so let it say so */ + wait_until(&thrd_pool, idle); + tpool_destroy(&thrd_pool); + return EXIT_FAILURE; + } } /* employer asks workers to work */ atomic_store(&thrd_pool.state, running); /* employer waits for the result of the job */ + bool complete = true; for (int i = 0; i < PRECISION; i++) { tpool_future_wait(futures[i]); - bbp_sum += *(double *)(futures[i]->result); + /* bbp returns NULL if it could not allocate its result */ + if (futures[i]->result) + bbp_sum += *(double *)(futures[i]->result); + else { + printf("Job %d produced no result.\n", i); + complete = false; + } tpool_future_destroy(futures[i]); } @@ -290,5 +313,5 @@ int main() wait_until(&thrd_pool, idle); tpool_destroy(&thrd_pool); printf("PI calculated with %d terms: %.15f\n", PRECISION, bbp_sum); - return 0; + return complete ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/examples/rmw_example_aba.c b/examples/rmw_example_aba.c index 398018a..580a708 100644 --- a/examples/rmw_example_aba.c +++ b/examples/rmw_example_aba.c @@ -279,7 +279,7 @@ static inline void wait_until(tpool_t *thrd_pool, int state) thrd_yield(); } -int main() +int main(void) { int bbp_args[PRECISION]; struct tpool_future *futures[PRECISION]; @@ -288,7 +288,7 @@ int main() tpool_t thrd_pool = { .initialized = ATOMIC_FLAG_INIT }; if (!tpool_init(&thrd_pool, N_THREADS)) { printf("failed to init.\n"); - return 0; + return EXIT_FAILURE; } /* employer asks workers to work */ atomic_store(&thrd_pool.state, running); @@ -300,15 +300,38 @@ int main() for (int i = 0; i < PRECISION; i++) { bbp_args[i] = i; futures[i] = add_job(&thrd_pool, bbp, &bbp_args[i]); + if (!futures[i]) { + printf("Failed to add job %d.\n", i); + /* Jobs handed out before this point own a future each, and a + * worker that claims one frees the job but not the future. Let + * them drain so those futures can be reclaimed here. + */ + atomic_store(&thrd_pool.state, running); + for (int j = 0; j < i; j++) { + tpool_future_wait(futures[j]); + tpool_future_destroy(futures[j]); + } + /* the pool is drained, so let it say so */ + wait_until(&thrd_pool, idle); + tpool_destroy(&thrd_pool); + return EXIT_FAILURE; + } } /* employer asks workers to work */ atomic_store(&thrd_pool.state, running); /* employer waits for the result of the job */ + bool complete = true; for (int i = 0; i < PRECISION; i++) { tpool_future_wait(futures[i]); - bbp_sum += *(double *)(futures[i]->result); + /* bbp returns NULL if it could not allocate its result */ + if (futures[i]->result) + bbp_sum += *(double *)(futures[i]->result); + else { + printf("Job %d produced no result.\n", i); + complete = false; + } tpool_future_destroy(futures[i]); } @@ -320,5 +343,5 @@ int main() wait_until(&thrd_pool, idle); tpool_destroy(&thrd_pool); printf("PI calculated with %d terms: %.15f\n", PRECISION, bbp_sum); - return 0; + return complete ? EXIT_SUCCESS : EXIT_FAILURE; } From 094a52def8aba49bedeb9641ceb44f7293af33ea Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:47:04 +0800 Subject: [PATCH 09/17] Make the ABA demonstration deterministic threadA looped while its compare-exchange kept succeeding, so it ran forever and the program only terminated because main slept two seconds and returned out from under it. Whether the fourth line was printed before the process died was a race, and nothing ordered A's load before B's updates, so B could finish first and leave no ABA to observe at all. Two flags pin the interleaving instead: B waits until A has read v, and A waits until B has changed it and changed it back. The four lines the manuscript quotes now come out on every run, and dropping the sleep takes 300 runs from five minutes to under two seconds. A compare-exchange that fails means the run demonstrates nothing, so it reports that and exits non-zero rather than printing a shorter story. Both creates are checked, since each thread spins on a flag only the other one sets. --- concurrency-primer.tex | 7 ++-- examples/simple_aba_example.c | 61 +++++++++++++++++++++++++++-------- 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/concurrency-primer.tex b/concurrency-primer.tex index 6a2f7b9..907bc84 100644 --- a/concurrency-primer.tex +++ b/concurrency-primer.tex @@ -932,8 +932,11 @@ \subsection{ABA problem} In the example provided, the ABA problem causes thread A to be unaware that variable \cc{v} has been altered. Since the comparison result indicates that \cc{v} is unchanged, \cc{v + 10} is swapped in. -Here, sleeping is only used to ensure the occurrence of the ABA problem. -In a real-world scenario, instead of sleeping, thread A could be paused by a context switch to another task, including preemption by a higher-priority task. +The two flags pin the interleaving down rather than leaving it to the scheduler: +\cc{a_has_read} holds thread B back until A has loaded \cc{v}, +since B finishing first would leave nothing for A to be misled about, +and \cc{b_done} holds A back until B has changed \cc{v} and changed it back. +In a real-world scenario, instead of this handshake, thread A could be paused by a context switch to another task, including preemption by a higher-priority task. This example seems harmless, but things can get nasty when atomic \textsc{RMW} operations are used in more complex data structures. In a broader context, the ABA problem occurs when changes occur between loading and comparing, but the comparing mechanism is unable to identify that the state of the target object is not the latest, yielding a false positive result. diff --git a/examples/simple_aba_example.c b/examples/simple_aba_example.c index 85df835..38adc24 100644 --- a/examples/simple_aba_example.c +++ b/examples/simple_aba_example.c @@ -1,18 +1,32 @@ #include +#include #include +#include #include -#include + atomic_int v = 42; +/* handshake flags: they pin the interleaving down instead of leaving it to + * the scheduler, as sleeping would. + */ +atomic_bool a_has_read = false, b_done = false; int threadA(void *args) { - int va; - do { - va = atomic_load(&v); - printf("A: v = %d\n", va); - /* Ensure thread B does something before comparing */ - thrd_sleep(&(struct timespec){ .tv_sec = 1 }, NULL); - } while (atomic_compare_exchange_strong(&v, &va, va + 10)); + (void)args; + + int va = atomic_load(&v); + printf("A: v = %d\n", va); + atomic_store(&a_has_read, true); + /* wait for B; a real program would be preempted here instead */ + while (!atomic_load(&b_done)) + thrd_yield(); + /* v was changed and changed back, so the comparison succeeds and A never + * learns that v changed at all. + */ + if (!atomic_compare_exchange_strong(&v, &va, va + 10)) { + printf("A: CAS failed, v = %d\n", atomic_load(&v)); + return 1; + } printf("A: v = %d\n", atomic_load(&v)); return 0; @@ -20,20 +34,39 @@ int threadA(void *args) int threadB(void *args) { + (void)args; + + while (!atomic_load(&a_has_read)) + thrd_yield(); + atomic_fetch_add(&v, 5); printf("B: v = %d\n", atomic_load(&v)); atomic_fetch_sub(&v, 5); printf("B: v = %d\n", atomic_load(&v)); + atomic_store(&b_done, true); return 0; } -int main() +int main(void) { thrd_t A, B; - thrd_create(&A, threadA, NULL); - thrd_create(&B, threadB, NULL); - /* Ensure all threads complete */ - thrd_sleep(&(struct timespec){ .tv_sec = 2 }, NULL); - return 0; + /* Each thread spins on a flag only the other one sets, so an unchecked + * thrd_create failure would leave the survivor spinning forever. + */ + if (thrd_create(&A, threadA, NULL) != thrd_success) { + printf("failed to create thread A.\n"); + return EXIT_FAILURE; + } + if (thrd_create(&B, threadB, NULL) != thrd_success) { + printf("failed to create thread B.\n"); + atomic_store(&b_done, true); /* release A, then reap it */ + thrd_join(A, NULL); + return EXIT_FAILURE; + } + + int result_a = 0; + thrd_join(A, &result_a); + thrd_join(B, NULL); + return result_a ? EXIT_FAILURE : EXIT_SUCCESS; } From cb7a24ba9c3331e8b5201879e48ef6b77675ea1e Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:47:05 +0800 Subject: [PATCH 10/17] Match the relaxed-ordering listing to the worker loop The listing is introduced as the worker from the thread pool section, but it referenced a job->args field that does not exist, detected idleness by testing that field against NULL rather than comparing against the sentinel, and called free on it: in the real program that argument points into an array on main's stack. It was also C code in a C++ listing environment. The ordering it showed was wrong in three ways as well. The load of the job pointer has to be an acquire, because the loop follows what it reads and a release on the successful compare-exchange orders the claiming thread's earlier work rather than the reads that follow it. The reload a failed compare-exchange performs needs the same. And the test for running announces a queue the employer has just filled, unlike the test for cancelled, so that load cannot be relaxed either: add_job publishes the head pointer atomically only for the first job of an empty queue, which leaves the store of running as the only edge carrying the rest. --- concurrency-primer.tex | 61 +++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 24 deletions(-) diff --git a/concurrency-primer.tex b/concurrency-primer.tex index 907bc84..e2622e4 100644 --- a/concurrency-primer.tex +++ b/concurrency-primer.tex @@ -1536,41 +1536,54 @@ \subsubsection{Relaxed} Relaxed operations are beneficial for managing flags shared between threads. For example, a worker thread in thread pool in \secref{rmw} might continuously run until it receives a cancelled signal: -\begin{cppcode} +\begin{ccode} while (1) { if (atomic_load_explicit(&thrd_pool->state, memory_order_relaxed) == cancelled) return EXIT_SUCCESS; - if (atomic_load_explicit(&thrd_pool->state, memory_order_relaxed) == running) { - // claim the job - job_t *job = atomic_load(&thrd_pool->head->prev); - while (!atomic_compare_exchange_weak_explicit(&thrd_pool->head->prev, &job, - job->prev, memory_order_release, - memory_order_relaxed)) { - } - if (job->args == NULL) { + /* acquire: unlike cancelled, this one announces a filled queue */ + if (atomic_load_explicit(&thrd_pool->state, memory_order_acquire) == running) { + /* claim the job */ + job_t *job = atomic_load_explicit(&thrd_pool->head->prev, + memory_order_acquire); + while (job != &thrd_pool->head->job && + !atomic_compare_exchange_weak_explicit(&thrd_pool->head->prev, + &job, job->prev, + memory_order_release, + memory_order_acquire)) + ; + if (job == &thrd_pool->head->job) { atomic_store(&thrd_pool->state, idle); + thrd_yield(); } else { - void *ret_value = job->func(job->args); - job->future->result = ret_value; + job->future->result = job->func(job->future->arg); atomic_flag_clear(&job->future->flag); - free(job->args); - free(job); // could cause dangling pointer in other threads + free(job); /* could cause dangling pointer in other threads */ } } else { - /* To auto run when jobs added, set status to running if job queue is not empty. - * As long as the producer is protected */ thrd_yield(); - continue; } -}; -\end{cppcode} -We do not care if the contents of the loop are rearranged around the load. +} +\end{ccode} +We do not care if the contents of the loop are rearranged around that load. Nothing bad will happen so long as \texttt{cancelled} is only used to tell the worker to exit, and not to ``announce'' any new data. - -Finally, relaxed loads are commonly used with \textsc{CAS} loops. -Continue the example above, -a \textsc{CAS} loop is performed to claim a job. -All of the loads can be relaxed as we do not need to enforce any order until we have successfully modified our value. +The very next line shows where that condition stops holding. +\cc{running} does announce data, because the employer fills the queue and only then switches the pool to it, +so that load is an acquire and pairs with the employer's store. +Everything the employer wrote beforehand, including the links it rewrote on jobs that were already queued, is ordered by that one pairing. +It is the whole reason the worker may then walk the queue at all, +and it holds only for as long as the employer confines its edits to a pool that is idle. + +Finally, relaxed loads are commonly used with \textsc{CAS} loops, +where a failed comparison means nothing more than ``try again'' +and no order needs enforcing until we have successfully modified our value. +The loop above is not one of those cases, which is why both of the reads it makes on \cc{head->prev} are acquires: +it follows the pointer it reads, taking \cc{job->prev} to compute the next candidate and dereferencing the job once the claim succeeds. +A release on the successful \textsc{CAS} only keeps the claiming thread's own earlier work from drifting past it; +it constrains nothing that comes afterwards, +so it cannot make the employer's writes visible to the reads that follow the claim. +That has to come from the reading side, and there are two reads here, both of which need it: +the initial load, and the reload that a failed compare-exchange performs, which is why the failure order is an acquire rather than relaxed. +A \textsc{CAS} loop that only re-reads a value it never follows can leave every load relaxed. \subsubsection{Acquire-Release} From fe9b56dbdc8bfa3b3b11bc1a2526447bbb0e80d7 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:47:05 +0800 Subject: [PATCH 11/17] Give each program its own make rule A single all target relinked all three programs on every invocation and hardcoded -mcx16, which does not exist outside x86-64 and fails the build outright elsewhere. Ask the compiler for its target rather than asking uname about the host, so a cross build is judged on what it is producing, and say so when the compiler cannot answer instead of quietly dropping the flag. -mcx16 does not make libatomic unnecessary either: some versions still route 16-byte atomic load and store through it, which is the link failure the manuscript footnotes. Probing beats guessing, and the probe carries the flags of the link it is deciding for. check now compares stdout against the text the manuscript quotes, since asserting only the exit status lets the book and its own programs drift apart while CI stays green. Flags are set with override, and the binaries depend on a stamp naming the toolchain, so caller flags compose and changing any of them rebuilds rather than leaving check to assert on stale binaries. --- examples/Makefile | 98 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 89 insertions(+), 9 deletions(-) diff --git a/examples/Makefile b/examples/Makefile index 3af7a89..a6bb2bd 100644 --- a/examples/Makefile +++ b/examples/Makefile @@ -1,12 +1,92 @@ -all: - $(CC) -Wall -o rmw_example rmw_example.c -pthread -lm - $(CC) -Wall -o rmw_example_aba rmw_example_aba.c -pthread -lm -mcx16 - $(CC) -Wall -o simple_aba_example simple_aba_example.c -pthread +# override, so that "make CFLAGS=-O2" adds to these rather than dropping the +# standard and the warnings the CI gate depends on. +override CFLAGS += -std=c11 -Wall -Wextra +LDLIBS := -pthread -lm + +# The ABA example claims a job with a 16-byte compare-and-swap. On x86-64 that +# is cmpxchg16b, which -mcx16 enables; the flag does not exist elsewhere. Ask +# the compiler rather than uname, so a cross build tests the target it is +# actually producing code for instead of the machine running make. +ABA_MACROS := $(shell $(CC) $(CFLAGS) $(CPPFLAGS) -dM -E -x c /dev/null 2>/dev/null) +ifeq ($(ABA_MACROS),) + # Say so rather than quietly building without cmpxchg16b, which turns into + # an undefined reference to __atomic_compare_exchange_16 at link time. + $(warning cannot query $(CC) for its target; building without -mcx16) +endif +ABA_CFLAGS := $(if $(findstring __x86_64__,$(ABA_MACROS)),-mcx16) + +# -mcx16 does not make libatomic unnecessary: some GCC versions still route +# 16-byte atomic load and store through it, which is the link failure the +# manuscript footnotes. Other toolchains inline everything and ship no +# libatomic at all, so probe rather than guess. The probe carries the same +# flags as the ABA build, otherwise it answers for the wrong target. +ABA_LDLIBS := $(shell printf 'int main(void){return 0;}' | \ + $(CC) $(CFLAGS) $(CPPFLAGS) $(ABA_CFLAGS) $(LDFLAGS) -x c - \ + $(LDLIBS) -latomic -o /dev/null 2>/dev/null && echo -latomic) + +BINS := rmw_example rmw_example_aba simple_aba_example + +# make compares timestamps, so "make CFLAGS=-O2" against an up-to-date tree +# would rebuild nothing and check would then assert on binaries built with +# other flags. Depend on a stamp whose name encodes the toolchain: change any +# of it and the stamp simply does not exist, which make repairs before the +# binaries in the same run. A stamp with a fixed name and rewritten contents +# does not work here, because make 3.81 stats prerequisites before running +# their recipes and would apply the change one invocation late. +# +# The compiler's own version banner is in the signature because upgrading the +# toolchain behind an unchanged CC would otherwise leave check validating +# binaries the previous compiler produced. +CC_ID := $(shell $(CC) --version 2>/dev/null | head -1) +TOOLCHAIN := $(CC)|$(CC_ID)|$(CFLAGS)|$(CPPFLAGS)|$(LDFLAGS)|$(LDLIBS)|$(ABA_CFLAGS)|$(ABA_LDLIBS) + +# A flag value may contain a single quote, which would otherwise close the +# quoting below and hand the shell something other than the signature. Escape +# it the usual way before interpolating. +sq := ' +TOOLCHAIN_SH := $(subst $(sq),$(sq)\$(sq)$(sq),$(TOOLCHAIN)) +STAMP := .toolchain-$(shell printf '%s' '$(TOOLCHAIN_SH)' | cksum | cut -d' ' -f1) + +all: $(BINS) + +$(STAMP): + @rm -f .toolchain* + @touch $@ + +rmw_example_aba: rmw_example_aba.c $(STAMP) + $(CC) $(CFLAGS) $(CPPFLAGS) $(ABA_CFLAGS) $(LDFLAGS) -o $@ $< $(LDLIBS) $(ABA_LDLIBS) + +%: %.c $(STAMP) + $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) -o $@ $< $(LDLIBS) + clean: - rm -f rmw_example rmw_example_aba simple_aba_example + rm -f $(BINS) .toolchain* + +# The manuscript quotes this output verbatim, so gate on the text and not +# just the exit status: otherwise the book and its own programs can drift +# apart with CI staying green. +PI_LINE := PI calculated with 100 terms: 3.141592653589793 +ABA_LINES := A: v = 42/B: v = 47/B: v = 42/A: v = 52/ + check: all - ./rmw_example - ./rmw_example_aba - ./simple_aba_example + @for p in rmw_example rmw_example_aba; do \ + out=$$(./$$p) || exit 1; \ + [ "$$out" = "$(PI_LINE)" ] || { \ + echo "$$p: expected '$(PI_LINE)', got '$$out'"; exit 1; }; \ + echo "$$p: $$out"; \ + done + @raw=$$(./simple_aba_example) || exit 1; \ + out=$$(printf '%s\n' "$$raw" | tr '\n' '/'); \ + [ "$$out" = "$(ABA_LINES)" ] || { \ + echo "simple_aba_example: expected '$(ABA_LINES)', got '$$out'"; exit 1; }; \ + echo "simple_aba_example: $$out" + +# Pinned to match .ci/check-format.sh: clang-format releases disagree about +# this style, and these files are printed verbatim in the book, so a reformat +# under a different version would churn the typeset listings. +CLANG_FORMAT ?= clang-format-20 + format: - clang-format -i --style=file *.c + $(CLANG_FORMAT) -i --style=file *.c + +.PHONY: all clean check format From 194878d92393ded17215d74fc944e6d3f55e3dc7 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:47:05 +0800 Subject: [PATCH 12/17] Rebuild the PDF when an example changes The examples reach the document through inputminted, which hands them to pygmentize rather than opening them as TeX inputs, so latexmk never records them as dependencies and examples/ appears nowhere in its database. Editing one left the PDF untouched and the book shipped a stale copy of its own code. Naming them as prerequisites is not enough on its own, because latexmk then consults that database and declares the PDF up to date, so force one pass when make has decided something changed. The list is read out of the document rather than globbed or spelled out here, since the inputminted directives are the only record of what the book actually embeds. --- Makefile | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 6b75e29..5edff2a 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,22 @@ lstlangarm.sty: wget https://raw.githubusercontent.com/sysprog21/arm-assembler-latex-listings/master/lstlangarm.sty -concurrency-primer.pdf: lstlangarm.sty lib/codeblock.tex concurrency-primer.tex - latexmk -lualatex -latexoption=-shell-escape concurrency-primer.tex +# The examples are pulled in with \inputminted, which reaches them through +# pygmentize rather than through TeX, so latexmk never records them as +# dependencies. Listing them here is what keeps an edited example from +# silently shipping stale in the PDF. +# +# Read out of the document rather than globbed or spelled out here: the +# \inputminted directives are the only place that records which examples the +# book actually embeds, so deriving the list from them cannot drift when one +# is added, renamed, or dropped. +EXAMPLES := $(shell sed -n 's|.*\\inputminted{c}{\./\(.*\)}|\1|p' concurrency-primer.tex) + +# -g because latexmk would otherwise consult its own dependency database and +# declare the PDF up to date. make already decided something changed, so force +# at least one pass; latexmk still handles the rerun-until-stable logic. +concurrency-primer.pdf: lstlangarm.sty lib/codeblock.tex concurrency-primer.tex $(EXAMPLES) + latexmk -g -lualatex -latexoption=-shell-escape concurrency-primer.tex # Clear the .DEFAULT_GOAL special variable, so that the following turns # to the first target after .DEFAULT_GOAL is not set. From e84c84e6468d4c54f0295a7950b0bff2b9a4d5c6 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:47:05 +0800 Subject: [PATCH 13/17] Clean and ignore what the build generates clean removed the minted cache under its minted v2 name while the current toolchain writes elsewhere, and left the LuaLaTeX intermediate behind, so roughly a megabyte survived it. The example binaries were neither cleaned nor ignored. --- .gitignore | 10 ++++++++++ Makefile | 9 +++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 98ffa27..5f3ecdd 100644 --- a/.gitignore +++ b/.gitignore @@ -6,5 +6,15 @@ concurrency-primer.pdf *.toc *.fls *.fdb_latexmk +*.xdv draftmark.tex lstlangarm.sty +.DS_Store + +# built by examples/Makefile +examples/rmw_example +examples/rmw_example_aba +examples/simple_aba_example + +# build-flag stamp from examples/Makefile +examples/.toolchain* diff --git a/Makefile b/Makefile index 5edff2a..5c21f79 100644 --- a/Makefile +++ b/Makefile @@ -24,9 +24,14 @@ concurrency-primer.pdf: lstlangarm.sty lib/codeblock.tex concurrency-primer.tex all: concurrency-primer.pdf +# _minted is the current minted cache directory; the suffixed name is what +# minted v2 used, kept so older working trees still get cleaned. clean: - rm -f *.dvi *.aux *.log *.ps *.pdf *.out *.bbl *.blg *.lof *.toc *.fdb_latexmk *.fls - rm -rf _minted-concurrency-primer + rm -f *.dvi *.xdv *.aux *.log *.ps *.pdf *.out *.bbl *.blg *.lof *.toc *.fdb_latexmk *.fls + rm -rf _minted _minted-concurrency-primer + $(MAKE) -C examples clean distclean: clean rm -f lstlangarm.sty + +.PHONY: all clean distclean From f25132db479987101bb64d8593a097d5d62d185f Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:47:05 +0800 Subject: [PATCH 14/17] Explain how to build the examples The programs need a libc with C11 threads, which rules out macOS, and the versioned example needs a double-width compare-and-swap. Neither was written down. Also note that the first build fetches a style file over the network. --- README.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/README.md b/README.md index e9ca4e5..7386db4 100644 --- a/README.md +++ b/README.md @@ -33,3 +33,21 @@ $ lualatex -halt-on-error -shell-escape concurrency-primer.tex until it no longer warns, "Label(s) may have changed. Rerun to get cross-references right." Enjoy a typeset `concurrency-primer.pdf`. + +The build fetches `lstlangarm.sty` with `wget`, so the first run needs network access. + +## Building the examples + +The programs under `examples/` are listed in the text and can be built and run on their own: +```shell +$ make -C examples check # build all three and run them +$ make -C examples format # reformat to examples/.clang-format +``` + +They use C11 threads (``), so they need a libc that provides them, such as glibc 2.28 or newer. +They do not build on macOS, whose libc omits C11 threads. + +`rmw_example_aba` claims a job with a double-width compare-and-swap. +On x86-64 the Makefile passes `-mcx16` to enable `cmpxchg16b`. +It also links against libatomic whenever the toolchain has it, on any architecture, +because some compilers still route 16-byte atomic loads and stores through libatomic even with `-mcx16`. From 986b4310b259b64751e8474dd963e76560c5bd0c Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:47:05 +0800 Subject: [PATCH 15/17] Move to current actions and make the cache usable checkout and cache were a major version behind. The TeX cache had no restore-keys, so its key changed on nearly every push and it never once restored; a prefix fallback is what makes it do anything. Add the minted output to it, which is the expensive part of the build, one pygmentize process per snippet. Both the key and that fallback carry the workflow hash, because the container digest lives there: minted names its entries by listing content alone, so after an image bump a bare prefix would serve every untouched listing from the previous pygments and the PDF would mix two highlighting styles. The style file stays out of the cache entirely, since the Makefile fetches it from a floating URL and a cached copy would pin the build to whichever revision landed first. Superseded pull request runs still cancel, but a push to main no longer does, since that run publishes the release and moves the tag. --- .github/workflows/main.yml | 41 ++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index bb5c915..1e7f399 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -11,10 +11,11 @@ on: permissions: contents: read -# Cancel superseded runs on the same ref to save minutes. +# Cancel superseded PR runs to save minutes, but never interrupt a push to +# main: that run publishes the release and moves the "latest" tag. concurrency: group: ${{ github.workflow }}-${{ github.ref }} - cancel-in-progress: true + cancel-in-progress: ${{ github.event_name == 'pull_request' }} defaults: run: @@ -27,7 +28,7 @@ jobs: timeout-minutes: 5 steps: - name: Checkout code - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Install clang-format run: | @@ -54,7 +55,7 @@ jobs: timeout-minutes: 30 steps: - name: Checkout code - uses: actions/checkout@v6 + uses: actions/checkout@v7 - name: Mark workspace safe for git # The texlive container runs as root; git refuses to operate on @@ -68,8 +69,8 @@ jobs: apt-get update -qq apt-get install -y --no-install-recommends wget ca-certificates - - name: Cache TeX intermediates - uses: actions/cache@v5 + - name: Cache TeX intermediates and minted highlights + uses: actions/cache@v6 with: path: | concurrency-primer.aux @@ -77,7 +78,27 @@ jobs: concurrency-primer.fls concurrency-primer.out concurrency-primer.toc - key: tex-${{ runner.os }}-${{ hashFiles('.github/workflows/main.yml', 'concurrency-primer.tex', 'Makefile', 'lib/**/*.tex', 'examples/**/*') }} + _minted + # The sources hash changes on nearly every push, so the prefix + # fallback below does the real work. A stale aux/toc costs latexmk + # one extra pass; _minted entries are content-addressed, so unmatched + # ones are ignored rather than reused wrongly. + # + # lstlangarm.sty stays out of the cache. The Makefile fetches it from + # a floating master URL, so a cached copy would pin the build to + # whatever revision landed in the cache first. + # The workflow file is in the hash because the texlive digest lives + # there: a new image means a new pygments, and minted keys its + # entries by listing content alone, so stale ones would still match + # and the PDF would mix two highlighting styles. + # The workflow hash is its own key segment, not just folded into the + # whole, so the fallback below can be scoped to it. A bare + # "tex-${{ runner.os }}-" prefix would happily restore _minted built + # by a different pygments after a digest bump, which is the mixed + # highlighting the exact key exists to prevent. + key: tex-${{ runner.os }}-${{ hashFiles('.github/workflows/main.yml') }}-${{ hashFiles('concurrency-primer.tex', 'Makefile', 'lib/**/*.tex', 'examples/**/*.c') }} + restore-keys: | + tex-${{ runner.os }}-${{ hashFiles('.github/workflows/main.yml') }}- - name: Build PDF run: make 2>&1 | tee build.log @@ -118,9 +139,9 @@ jobs: contents: write steps: - name: Checkout code - uses: actions/checkout@v6 - with: - fetch-depth: 0 + # A shallow clone suffices; force-moving the tag to HEAD walks no + # history, and the remote already has that commit. + uses: actions/checkout@v7 - name: Download PDF artifact uses: actions/download-artifact@v8 From d85ad4f32a64283ed1c61e989cae5e2cde45d121 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:47:05 +0800 Subject: [PATCH 16/17] Add standalone scripts for the C checks One script per check, each runnable by hand rather than only through CI, and driven by the file list git already has. Adds a trailing-newline check, a sweep for banned functions and hardcoded secrets, and cppcheck alongside the formatting gate. clang-format is pinned to one release. These files are printed verbatim in the book, so the accepted style has to be a single version rather than whichever major a machine happens to carry, and the script says so instead of silently reformatting to a different one. The security sweep folds spliced lines before scanning and replaces comments with a space, matching translation phases 2 and 3, so that a banned name written in prose cannot fail the build and a definition split by a comment cannot hide from it. Quoted literals are copied out untouched, since a string holding a comment opener would otherwise swallow the code after it and the secret pattern matches on literals anyway. cppcheck runs with warnings enabled on top of the error checks that always run. That does not exempt the use-after-free the manuscript keeps on purpose; cppcheck simply does not see it, because the free and the stale read are in different threads rather than along one path through a function. --- .ci/check-cppcheck.sh | 29 +++++++++++ .ci/check-format.sh | 39 +++++++++++++++ .ci/check-newline.sh | 23 +++++++++ .ci/check-security.sh | 109 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 200 insertions(+) create mode 100755 .ci/check-cppcheck.sh create mode 100755 .ci/check-format.sh create mode 100755 .ci/check-newline.sh create mode 100755 .ci/check-security.sh diff --git a/.ci/check-cppcheck.sh b/.ci/check-cppcheck.sh new file mode 100755 index 0000000..01191f3 --- /dev/null +++ b/.ci/check-cppcheck.sh @@ -0,0 +1,29 @@ +#!/usr/bin/env bash +# Static analysis over the C examples. +# +# --enable=warning adds to the error-severity checks rather than selecting +# them: those always run. So this does not exempt the use-after-free the +# manuscript keeps on purpose, and no suppression is needed for it either. +# cppcheck does not see it, because the free and the stale read are in +# different threads rather than along one path through a function. Style and +# portability sweeps stay off, since those would report the deliberate +# defects the book is built around. +set -e -u -o pipefail + +mapfile -t SOURCES < <(git ls-files -z -- 'examples/*.c' | tr '\0' '\n') + +if [ ${#SOURCES[@]} -eq 0 ]; then + echo "No tracked C source files found." + exit 0 +fi + +timeout 120 cppcheck \ + --platform=unix64 \ + --enable=warning \ + --max-configs=1 --error-exitcode=1 --inline-suppr \ + --suppress=checkersReport --suppress=unmatchedSuppression \ + --suppress=missingIncludeSystem --suppress=noValidConfiguration \ + --suppress=normalCheckLevelMaxBranches \ + --suppress=preprocessorErrorDirective \ + -D_GNU_SOURCE -D__linux__ \ + "${SOURCES[@]}" diff --git a/.ci/check-format.sh b/.ci/check-format.sh new file mode 100755 index 0000000..a9298d0 --- /dev/null +++ b/.ci/check-format.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +# Verify clang-format conformance for the C examples the book embeds. +# +# The version is pinned because clang-format releases disagree about this +# style, and the examples are printed verbatim in the PDF: a reformat that +# only some contributors reproduce would churn the typeset listings. +set -e -u -o pipefail + +if [ -z "${CLANG_FORMAT:-}" ]; then + if command -v clang-format-20 >/dev/null 2>&1; then + CLANG_FORMAT="clang-format-20" + else + echo "Error: clang-format-20 is required (other versions differ in style)" >&2 + exit 1 + fi +fi + +# One scratch file removed on any exit, rather than one per iteration that a +# mid-loop failure would leave behind. +expected=$(mktemp) +trap 'rm -f "$expected"' EXIT + +ret=0 +while IFS= read -r -d '' file; do + # Keep clang-format's own diagnostics: a parse or config error otherwise + # aborts the run with a bare exit code and nothing to act on. + if ! err=$("$CLANG_FORMAT" --style=file "$file" 2>&1 >"$expected"); then + echo "Error: clang-format failed on $file" >&2 + [ -n "$err" ] && printf '%s\n' "$err" >&2 + ret=1 + continue + fi + [ -n "$err" ] && printf '%s\n' "$err" >&2 + if ! diff -u -p --label="$file" --label="expected coding style" "$file" "$expected"; then + ret=1 + fi +done < <(git ls-files -z -- 'examples/*.c') + +exit $ret diff --git a/.ci/check-newline.sh b/.ci/check-newline.sh new file mode 100755 index 0000000..e304970 --- /dev/null +++ b/.ci/check-newline.sh @@ -0,0 +1,23 @@ +#!/usr/bin/env bash +# Every tracked C example must end with a newline. +# +# minted reproduces the file as-is, so a missing final newline shows up in the +# typeset listing as well as annoying every diff that touches the last line. +set -e -u -o pipefail + +ret=0 +while IFS= read -r -d '' f; do + if [ ! -s "$f" ]; then + # tail -c1 of an empty file is empty, which would otherwise read as a + # correctly terminated file. + echo "Warning: empty file $f" + ret=1 + elif file --mime-encoding "$f" | grep -qv binary; then + if [ -n "$(tail -c1 <"$f")" ]; then + echo "Warning: No newline at end of file $f" + ret=1 + fi + fi +done < <(git ls-files -z -- 'examples/*.c') + +exit $ret diff --git a/.ci/check-security.sh b/.ci/check-security.sh new file mode 100755 index 0000000..15495e6 --- /dev/null +++ b/.ci/check-security.sh @@ -0,0 +1,109 @@ +#!/usr/bin/env bash +# Reject unsafe C idioms in the examples the book prints. +# +# Readers copy this code. The concurrency defects the manuscript keeps on +# purpose are discussed in the text; an unbounded string function smuggled in +# by a later edit would not be, so catch that class here. +set -u -o pipefail + +failed=0 + +banned='(^|[^[:alnum:]_])(gets|sprintf|vsprintf|strcpy|stpcpy|strcat|atoi|atol|atoll|atof|mktemp|tmpnam|tempnam)[[:space:]]*\(' +secrets='(password|secret|api_key|private_key|token)[[:space:]]*=[[:space:]]*"[^"]+' +# undef of either guard disables hardening whatever the value; only the define +# side needs to pin _FORTIFY_SOURCE to 0 to be worth reporting. +dangerous_pp='#[[:space:]]*(undef[[:space:]]+(_FORTIFY_SOURCE|__SSP__)|define[[:space:]]+(_FORTIFY_SOURCE[[:space:]]+0|__SSP__))' + +# Blank out comments while keeping one output line per input line, so prose +# that names a banned function cannot fail the build and the line numbers +# reported below still refer to the real file. +# +# Two details are what make this safe rather than merely convenient. A comment +# becomes a space, as it does in translation phase 3, because deleting it would +# splice the tokens on either side and "_FORTIFY_SOURCE/**/0" would stop +# looking like a definition of it. And quoted literals are copied out +# untouched, honouring backslash escapes, because a string holding "/*" would +# otherwise put the scanner in a comment and swallow the code after it. +# Literals have to survive anyway: the secret pattern below matches on them. +strip_comments() { + awk ' + BEGIN { in_block = 0; buf = ""; held = 0 } + { + # Translation phase 2 first: a backslash-newline is deleted before + # anything is tokenized, whatever precedes it. Doing this ahead of the + # scan is what keeps a spliced string, a spliced // comment, and a + # backslash run at end of line from being read differently here than + # by the compiler. Blank lines stand in for the lines folded away, so + # reported line numbers keep pointing into the file. + buf = buf $0 + if (substr($0, length($0), 1) == "\\") { + sub(/\\$/, "", buf) + held++ + next + } + emit() + } + END { if (buf != "" || held > 0) emit() } + + function emit( k) { + print scan(buf) + for (k = 0; k < held; k++) print "" + buf = ""; held = 0 + } + + # in_block is deliberately global: a block comment spans logical lines. + # A literal cannot, once phase 2 has run, so quote state is local. + function scan(s, out, i, n, ch, nx, in_str, in_chr) { + out = ""; i = 1; n = length(s); in_str = 0; in_chr = 0 + while (i <= n) { + ch = substr(s, i, 1); nx = substr(s, i + 1, 1) + if (in_block) { + if (ch == "*" && nx == "/") { in_block = 0; i += 2 } else { i++ } + continue + } + if (in_str || in_chr) { + out = out ch + if (ch == "\\") { out = out nx; i += 2; continue } + if (in_str && ch == "\"") in_str = 0 + if (in_chr && ch == "'"'"'") in_chr = 0 + i++ + continue + } + if (ch == "\"") { in_str = 1; out = out ch; i++; continue } + if (ch == "'"'"'") { in_chr = 1; out = out ch; i++; continue } + if (ch == "/" && nx == "*") { in_block = 1; out = out " "; i += 2; continue } + if (ch == "/" && nx == "/") break + out = out ch; i++ + } + return out + }' "$1" +} + +# Here-strings rather than "echo ... | grep -q": grep exits on first match, the +# writer takes SIGPIPE, and under pipefail the condition then reads as false, +# which would silently pass a file that does contain a banned call. +while IFS= read -r -d '' f; do + code=$(strip_comments "$f") + + if grep -qE "$banned" <<<"$code"; then + echo "Banned function in $f:" + grep -nE "$banned" <<<"$code" + failed=1 + fi + if grep -iqE "$secrets" <<<"$code"; then + echo "Possible hardcoded secret in $f:" + grep -inE "$secrets" <<<"$code" + failed=1 + fi + if grep -qE "$dangerous_pp" <<<"$code"; then + echo "Dangerous preprocessor directive in $f:" + grep -nE "$dangerous_pp" <<<"$code" + failed=1 + fi +done < <(git ls-files -z -- 'examples/*.c') + +if [ $failed -eq 0 ]; then + echo "Security checks passed." +fi + +exit $failed From 9a714cffbe13b2a7ce73951cbb1eada3ca5e3455 Mon Sep 17 00:00:00 2001 From: Jim Huang Date: Fri, 7 Aug 2026 11:47:05 +0800 Subject: [PATCH 17/17] Build and run examples, and gate the release The job only checked formatting, so an example that no longer compiled, no longer terminated, or no longer printed what the manuscript quotes would still be typeset into the PDF and published. Run the checks, and make the release depend on that job: latexmk typesets the examples from source and succeeds either way, so nothing else can catch it. The toolchain is installed rather than taken from the runner image, which means the image no longer decides the accepted style, and the runner is pinned so what does decide it is reproducible. The release also accepts a manual run against main. Runs get lost to queue backlogs and cancellations, and without this the only way to publish that commit is to push another one. The ref check is what keeps it honest: dispatching on any other branch still skips the job, exactly as a pull request does. --- .github/workflows/main.yml | 67 +++++++++++++++++++++++++++++--------- 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1e7f399..7530c72 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -22,28 +22,53 @@ defaults: shell: bash jobs: - format: - name: Check C formatting (clang-format) - runs-on: ubuntu-latest - timeout-minutes: 5 + examples: + name: Check C examples + # Pinned rather than ubuntu-latest so the toolchain this job installs is + # reproducible; each check lives in .ci/ so it can be run locally too. + runs-on: ubuntu-24.04 + timeout-minutes: 10 steps: - name: Checkout code uses: actions/checkout@v7 - - name: Install clang-format + - name: Cache apt packages + uses: actions/cache@v6 + with: + path: ~/apt-cache + key: apt-lint-${{ runner.os }}-${{ hashFiles('.github/workflows/main.yml') }} + + - name: Install tools + # clang-format-20 specifically: the accepted style is whatever that + # release produces, and the image's default clang-format is a + # different major version. run: | + mkdir -p ~/apt-cache sudo apt-get update -qq - sudo apt-get install -y --no-install-recommends clang-format + sudo apt-get install -y -o Dir::Cache::Archives="$HOME/apt-cache" \ + clang-format-20 cppcheck - - name: Format examples/*.c with examples/.clang-format - run: make -C examples format + - name: Check trailing newline + run: .ci/check-newline.sh - - name: Fail if formatting produced changes + - name: Check clang-format run: | - if ! git diff --exit-code -- examples/; then + .ci/check-format.sh || { echo "::error::examples/*.c is not formatted; run 'make -C examples format' and commit the result." exit 1 - fi + } + + - name: Security checks + run: .ci/check-security.sh + + - name: Static analysis (cppcheck) + run: .ci/check-cppcheck.sh + + - name: Build and run the examples + # The book embeds these files verbatim, so a program that no longer + # compiles, no longer terminates, or no longer prints what the + # manuscript quotes ships straight into the PDF. + run: make -C examples check build: name: Build PDF @@ -129,10 +154,20 @@ jobs: release: name: Publish latest release - needs: build - # Release only on direct push to main. PRs run the build job above for - # validation but never reach this job, so no "main" tag is ever created. - if: github.event_name == 'push' && github.ref == 'refs/heads/main' + # Both, not just build: latexmk typesets the examples from source and + # succeeds whether or not they compile, so only the examples job can stop + # a broken program from shipping inside the PDF. + needs: [build, examples] + # Release on a direct push to main, or on a manual run against main. PRs + # run the build job above for validation but never reach this job, so no + # "main" tag is ever created. + # + # workflow_dispatch is the recovery path: when a push's run is lost, and + # a queue backlog or a cancelled run is enough to lose one, there is + # otherwise no way to publish that commit short of pushing another. + if: >- + (github.event_name == 'push' || github.event_name == 'workflow_dispatch') + && github.ref == 'refs/heads/main' runs-on: ubuntu-latest timeout-minutes: 10 permissions: @@ -162,7 +197,7 @@ jobs: Source commit: ${{ github.sha }} - name: Move latest tag to current commit - # On the first publish, action-gh-release can create `latest` at the + # On the first publish, action-gh-release can create "latest" at the # current commit via target_commitish. On later publishes, the Releases # API updates assets/body but leaves an existing tag in place, so move # the ref only after the upload succeeds.