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 diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index bb5c915..7530c72 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -11,38 +11,64 @@ 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: 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@v6 + uses: actions/checkout@v7 + + - name: Cache apt packages + uses: actions/cache@v6 + with: + path: ~/apt-cache + key: apt-lint-${{ runner.os }}-${{ hashFiles('.github/workflows/main.yml') }} - - name: Install clang-format + - 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 @@ -54,7 +80,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 +94,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 +103,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 @@ -108,19 +154,29 @@ 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: 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 @@ -141,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. 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 6b75e29..5c21f79 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. @@ -10,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 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`. diff --git a/concurrency-primer.tex b/concurrency-primer.tex index 2f85c48..e2622e4 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. @@ -927,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. @@ -939,8 +947,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} @@ -1527,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} 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 diff --git a/examples/rmw_example.c b/examples/rmw_example.c index d5b0c68..024e12a 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; @@ -77,6 +81,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 +95,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 +102,7 @@ static int worker(void *args) /* worker is idle */ thrd_yield(); } - }; + } return EXIT_SUCCESS; } @@ -109,12 +117,20 @@ 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; } - 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"); + free(thrd_pool->pool); + atomic_flag_clear(&thrd_pool->initialized); return false; } @@ -128,22 +144,46 @@ 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; } 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; } @@ -180,13 +220,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; @@ -200,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]; @@ -209,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); @@ -221,20 +270,48 @@ 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]); } - /* 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; + return complete ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/examples/rmw_example_aba.c b/examples/rmw_example_aba.c index 5449bf2..580a708 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; @@ -86,6 +91,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 +110,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 +118,7 @@ static int worker(void *args) /* worker is idle */ thrd_yield(); } - }; + } return EXIT_SUCCESS; } @@ -125,12 +133,21 @@ 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; } - 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"); + free(thrd_pool->pool); + atomic_flag_clear(&thrd_pool->initialized); return false; } @@ -145,22 +162,46 @@ 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; } 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; } @@ -197,15 +238,35 @@ 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. + */ + 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 (thrd_pool->head->prev == &thrd_pool->head->job) { - thrd_pool->head->prev = job; - thrd_pool->head->version += 1; + + 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; } @@ -218,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]; @@ -227,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); @@ -239,20 +300,48 @@ 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]); } - /* 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; + return complete ? EXIT_SUCCESS : EXIT_FAILURE; } 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; }