ci: Detect TSan lock-order inversions and add a TSan CI job

tsan.supp turned lock-order checking off wholesale: deadlock:pthread_create,
deadlock:pthread_rwlock_rdlock and deadlock:boost::asio name locking
primitives rather than source files, so between them they covered every
std::shared_mutex read lock and any inversion reached through a strand or a
thread start, and detect_deadlocks defaults to true. Seven more named source
files, among them ValidatorList.cpp, ValidatorSite.cpp and Manifest.cpp. Drop
all ten, because the job cannot fail a build, so a suppression here costs a
finding and buys nothing. The Manifest.cpp line alone was hiding a two-thread
deadlock between ManifestCache::save() and the validator list, which a run
without it reported 52 times on the shutdown path. Also repoint eight
patterns at files that moved into libxrpl and the gtest tree, and empty
sanitizer-ignorelist.txt of 24 entries that never matched anything, either
through suppression syntax a clang ignorelist never consults or through a
glob with no leading star. Both rules are now written down in
docs/build/sanitizers.md.

Add ubuntu-clang-debug-amd64-tsan to the Linux matrix, in a config of its
own so TSan stays on clang and Debug, and give the matrix a third tier so
it runs at night rather than on every labelled pull request. A Linux config
may now declare "extended", which holds it out of both the minimal and the
full matrix; generate.py emits those configs only for --extended, which the
workflow passes on a schedule or a manual run. The name says which matrix a
config belongs to, like "minimal", rather than naming a trigger, because the
trigger set already grew from the schedule to manual runs and a config
property outlives that. It avoids "maximal", which reads as a synonym for
the full matrix it is meant to be larger than. It is a whole hour of
runner time, which is too much to spend per pull request, and it reports
nothing back anyway, because the workflow appends exitcode=0 to
TSAN_OPTIONS. That belongs in the workflow rather than in
runtime-tsan-options.txt, which the documented local command also reads and
which must keep failing on a finding.

Cap the test jobs at 4 under TSan, which is measured rather than chosen.
One per core is 30 on the current runner and twice starved it until it lost
contact with the server; 12 was killed by the OOM killer with code 137
before a single suite finished; 4 completes the suite. The machine has 32
cores and no swap, so nothing absorbs the peak, and the runner is one pod
among several on a node, so the ceiling is not ours alone.

Check that a build carries the instrumentation it asked for, by the __asan,
__tsan and __ubsan symbols in the binary, since instrumented code
references its runtime however that runtime is linked. The version string
is checked too, but cannot stand alone, because cmake sets the SANITIZERS
macro separately from the flags. Widen the voidstar step for this rather
than add a second one, and add ASAN_ENABLED, TSAN_ENABLED and UBSAN_ENABLED
so a step needing one sanitizer does not parse the list. Define XRPL_ASAN,
XRPL_TSAN and XRPL_UBSAN so a test can skip when its sanitizer is inactive,
and drop the -Dcoverage_test_parallelism example from BUILD.md, which
neither cmake nor conanfile.py defines.

.github/scripts/strategy-matrix/generate.py

@@ -60,6 +60,11 @@ def get_cmake_args(build_type: str, extra_args: str) -> str:
 # Every config must declare 'minimal'. Minimal configs form the reduced matrix
 # built for pull requests by default; the full matrix adds the rest.
 #
+# A Linux config may instead declare 'extended'. Neither the minimal nor the full
+# matrix includes such a config; only the extended matrix does, which the nightly
+# schedule and a manual run ask for. Use it for a config too expensive to run per
+# pull request. It is Linux only because nothing else needs it yet.
+#
 # Configs may also opt into 'benchmark' to smoke-run the benchmarks, or carry a
 # 'package' map to be packaged as well. Note that either applies to every entry
 # a config expands into, so only set them on configs that expand to a single
@@ -94,6 +99,7 @@ class LinuxConfig:
     build_type: list[str]
     arch: list[str]
     minimal: bool
+    extended: bool = False
     benchmark: bool = False  # if true, smoke-run the benchmarks after testing
     sanitizers: list[str] = dataclasses.field(default_factory=list)
     suffix: str = ""
@@ -215,12 +221,22 @@ _ARCHS: dict[str, Architecture] = {
 }
 
 
-def expand_linux_matrix(linux: LinuxFile, minimal: bool) -> list[MatrixEntry]:
+def expand_linux_matrix(
+    linux: LinuxFile, minimal: bool, extended: bool = False
+) -> list[MatrixEntry]:
     """Expand a LinuxFile into a flat list of matrix entries.
 
     Each config entry is expanded over the cross-product of its
     compiler, build_type, sanitizers, and architecture lists. When 'minimal' is
     true, only configs flagged as minimal are included.
+
+    @param linux The parsed linux.json.
+    @param minimal Emit only the configs flagged 'minimal'.
+    @param extended Emit the configs flagged 'extended'. Both the minimal and
+        the full matrix leave those out, so this is the only way to get them.
+    @return One entry per combination the surviving configs expand into.
+    @note Do not set both 'minimal' and 'extended'. The command line rejects the
+        pair, but a direct caller gets the minimal matrix rather than an error.
     """
     entries: list[MatrixEntry] = []
 
@@ -228,6 +244,8 @@ def expand_linux_matrix(linux: LinuxFile, minimal: bool) -> list[MatrixEntry]:
         for cfg in configs:
             if minimal and not cfg.minimal:
                 continue
+            if not extended and cfg.extended:
+                continue
             # An empty sanitizers list means "one entry with no sanitizer".
             effective_sanitizers = cfg.sanitizers or [""]
             effective_archs = {arch: _ARCHS[arch] for arch in cfg.arch}
@@ -367,7 +385,12 @@ if __name__ == "__main__":
         help="Emit the Linux packaging matrix instead of the build/test matrix.",
         action="store_true",
     )
-    parser.add_argument(
+    # Each flag picks a matrix size, and the sizes nest: minimal is a subset of
+    # the full matrix, which is a subset of extended. So one flag narrows and the
+    # other widens the same default, and asking for both has no answer. Argparse
+    # rejects the pair rather than letting the filters intersect into a surprise.
+    size = parser.add_mutually_exclusive_group()
+    size.add_argument(
         "-m",
         "--minimal",
         help="Emit only the minimal matrix (the configs flagged 'minimal'), "
@@ -375,6 +398,14 @@ if __name__ == "__main__":
         "emitted.",
         action="store_true",
     )
+    size.add_argument(
+        "-x",
+        "--extended",
+        help="Emit the extended matrix: the full one plus the configs flagged "
+        "'extended', which no other matrix includes. Used for the nightly "
+        "schedule and for a manual run.",
+        action="store_true",
+    )
     args = parser.parse_args()
 
     matrix: list[MatrixEntry] | list[PackagingEntry] = []
@@ -388,7 +419,7 @@ if __name__ == "__main__":
     else:
         if args.config in ("linux", None):
             matrix += expand_linux_matrix(
-                LinuxFile.load(THIS_DIR / "linux.json"), args.minimal
+                LinuxFile.load(THIS_DIR / "linux.json"), args.minimal, args.extended
             )
         if args.config in ("macos", None):
             matrix += expand_platform_matrix(

.github/scripts/strategy-matrix/linux.json

@@ -38,6 +38,14 @@
         "minimal": false,
         "sanitizers": ["address", "undefinedbehavior"]
       },
+      {
+        "compiler": ["clang"],
+        "build_type": ["Debug"],
+        "arch": ["amd64"],
+        "minimal": false,
+        "extended": true,
+        "sanitizers": ["thread"]
+      },
 
       {
         "compiler": ["clang"],

.github/workflows/reusable-build-test-config.yml

@@ -117,6 +117,12 @@ jobs:
       VOIDSTAR_ENABLED: ${{ contains(inputs.cmake_args, '-Dvoidstar=ON') }}
       VALIDATOR_KEYS_ENABLED: ${{ contains(inputs.cmake_args, '-Dvalidator_keys=ON') }}
       SANITIZERS_ENABLED: ${{ inputs.sanitizers != '' }}
+      # One flag per sanitizer, so that a step needing only one of them does not
+      # have to parse the list. conan/profiles/sanitizers rejects any name that is
+      # not one of these three, so an unrecognized one never reaches a step.
+      ASAN_ENABLED: ${{ contains(inputs.sanitizers, 'address') }}
+      TSAN_ENABLED: ${{ contains(inputs.sanitizers, 'thread') }}
+      UBSAN_ENABLED: ${{ contains(inputs.sanitizers, 'undefinedbehavior') }}
       # The binaries reusable-package.yml consumes. A private repository skips
       # them except on a tag push, which is what produces its release packages.
       PACKAGING_ARTIFACTS_ENABLED: ${{ github.event.repository.visibility == 'public' || startsWith(github.ref, 'refs/tags/') }}
@@ -221,7 +227,12 @@ jobs:
               ASAN_OPTS="${ASAN_OPTS}:alloc_dealloc_mismatch=0"
           fi
           echo "ASAN_OPTIONS=${ASAN_OPTS}" >>${GITHUB_ENV}
-          echo "TSAN_OPTIONS=include=${SUPP}/runtime-tsan-options.txt:suppressions=${SUPP}/tsan.supp" >>${GITHUB_ENV}
+          # exitcode=0 is set here rather than in runtime-tsan-options.txt,
+          # because that file is also what the documented local command reads,
+          # and a local run must still exit nonzero on a finding. TSan exits 66
+          # on its own once it has reported anything, which would fail the job.
+          TSAN_OPTS="include=${SUPP}/runtime-tsan-options.txt:suppressions=${SUPP}/tsan.supp"
+          echo "TSAN_OPTIONS=${TSAN_OPTS}:exitcode=0" >>${GITHUB_ENV}
           echo "UBSAN_OPTIONS=include=${SUPP}/runtime-ubsan-options.txt:suppressions=${SUPP}/ubsan.supp" >>${GITHUB_ENV}
           echo "LSAN_OPTIONS=include=${SUPP}/runtime-lsan-options.txt:suppressions=${SUPP}/lsan.supp" >>${GITHUB_ENV}
 
@@ -329,11 +340,102 @@ jobs:
               exit 1
           fi
 
+      # Catch a build configured without the instrumentation it was asked for,
+      # which would otherwise run every test, pass, and report nothing.
+      #
+      # A sanitizer is checked by the symbols the binary carries, because
+      # instrumented code calls into its runtime, so those references are present
+      # whether the runtime is linked statically or as a shared library. That is
+      # what proves the -fsanitize flags reached the compiler. The version string
+      # cannot: cmake sets the SANITIZERS macro separately from those flags, so it
+      # names a sanitizer in a binary carrying none. It is checked as well, on top
+      # rather than instead, because BuildInfo.cpp is what a bug report quotes and
+      # it should agree with the binary.
+      #
+      # Asking each runtime to print its flag list was tried and dropped: GCC's
+      # libubsan ignores UBSAN_OPTIONS=help=1, so it called an instrumented gcc
+      # UBSan build uninstrumented.
+      #
+      # voidstar is a linked library rather than instrumentation, so the version
+      # string is all there is to check for it.
+      #
+      # The version call's exit code is ignored on purpose. ASan leaves leak
+      # reporting on and UBSan sets halt_on_error, either of which can end it
+      # nonzero on a healthy binary. Only the text decides, and it is printed.
       - name: Verify presence of instrumentation (Linux)
-        if: ${{ runner.os == 'Linux' && env.VOIDSTAR_ENABLED == 'true' }}
+        if: ${{ runner.os == 'Linux' && (env.VOIDSTAR_ENABLED == 'true' || env.SANITIZERS_ENABLED == 'true') }}
         working-directory: ${{ env.BUILD_DIR }}
         run: |
-          ./xrpld --version | grep libvoidstar
+          version="$(./xrpld --version 2>&1 || true)"
+          echo "${version}"
+          status=0
+
+          named() {
+              if grep -q "$2" <<<"${version}"; then
+                  echo "${1}: the version string names '${2}'."
+              else
+                  echo "${1} is enabled, but the version string does not name '${2}'."
+                  status=1
+              fi
+          }
+
+          if [ "${SANITIZERS_ENABLED}" = 'true' ]; then
+              # Whichever of these the image ships. Stopping here beats a check
+              # that quietly stops checking.
+              symbols="$(mktemp)"
+              if command -v nm >/dev/null 2>&1; then
+                  nm ./xrpld >"${symbols}" 2>/dev/null || true
+              elif command -v readelf >/dev/null 2>&1; then
+                  readelf -sW ./xrpld >"${symbols}" 2>/dev/null || true
+              elif command -v objdump >/dev/null 2>&1; then
+                  objdump -t ./xrpld >"${symbols}" 2>/dev/null || true
+              else
+                  echo 'Found none of nm, readelf or objdump, so the binary cannot be inspected.'
+                  exit 1
+              fi
+              if [ ! -s "${symbols}" ]; then
+                  echo 'Read no symbols from ./xrpld, so the instrumentation cannot be confirmed.'
+                  exit 1
+              fi
+
+              # Each pattern names the calls a sanitizer plants at an instrumented
+              # operation, rather than the ones that only prove its runtime is
+              # present. __asan_init and __tsan_init appear as soon as the flags
+              # reach the linker, so matching those would pass a build whose
+              # compile step never saw them. __tsan_func_entry is no better: it
+              # survives an ignorelist, while the read and write calls do not.
+              #
+              # How much this buys depends on how the runtime is linked, and both
+              # kinds are in the matrix. GCC links libasan and libubsan
+              # dynamically, so an instrumented call site is the only thing that
+              # can put these symbols in the binary. Clang links the TSan runtime
+              # statically, so the runtime's own definitions are there either way
+              # and this narrows to a check that the sanitizer reached the link.
+              # Nothing cheap distinguishes the two cases, and the flags come from
+              # one cmake variable, so losing only the compile half is unlikely.
+              instrumented() {
+                  if grep -qE "$2" "${symbols}"; then
+                      echo "${1}: the binary calls into its runtime, matching '${2}'."
+                  else
+                      echo "${1} is enabled, but no instrumented call site matches '${2}'."
+                      status=1
+                  fi
+              }
+
+              [ "${ASAN_ENABLED}" = 'true' ] &&
+                  instrumented address '__asan_report|__asan_memcpy|__asan_stack_malloc'
+              [ "${TSAN_ENABLED}" = 'true' ] &&
+                  instrumented thread '__tsan_read|__tsan_write|__tsan_atomic'
+              [ "${UBSAN_ENABLED}" = 'true' ] &&
+                  instrumented undefinedbehavior '__ubsan_handle'
+          fi
+
+          [ "${VOIDSTAR_ENABLED}" = 'true' ] && named voidstar libvoidstar
+          [ "${ASAN_ENABLED}" = 'true' ] && named address ASAN
+          [ "${TSAN_ENABLED}" = 'true' ] && named thread TSAN
+          [ "${UBSAN_ENABLED}" = 'true' ] && named undefinedbehavior UBSAN
+
+          exit "${status}"
 
       - name: Run the separate tests
         if: ${{ !inputs.build_only }}
@@ -350,6 +452,16 @@ jobs:
           # Coverage builds are slower due to instrumentation; use fewer parallel jobs to avoid flakiness
           [ "$COVERAGE_ENABLED" = "true" ] && BUILD_NPROC=$((BUILD_NPROC - 2))
 
+          # TSan keeps shadow memory for every byte a process touches, so each job
+          # costs several times what it does uninstrumented, and too many at once
+          # exhausts the machine. Cap the count rather than subtract from it,
+          # because the ceiling is total memory and does not scale with the core
+          # count. Raise it only against a run, and expect one of two failures:
+          # the OOM killer ends the process with code 137 before any suite
+          # finishes, or, worse, the runner loses contact with the server and the
+          # job stops with no log and no failing step.
+          [ "$TSAN_ENABLED" = "true" ] && BUILD_NPROC=4
+
           # The resolver/preload workaround is only correct for the ASan build:
           # a regular build doesn't hit the __dn_expand interceptor bug, and must
           # NOT have libasan injected. So only preload when xrpld is ASan-built.

.github/workflows/reusable-strategy-matrix.yml

@@ -39,4 +39,9 @@ jobs:
           # labeled "Ready to merge" or "Full CI build". Any other event (merge
           # queue, push, schedule, manual dispatch) runs the full matrix.
           GENERATE_MINIMAL: ${{ (github.event_name == 'pull_request' && !contains(github.event.pull_request.labels.*.name, 'Ready to merge') && !contains(github.event.pull_request.labels.*.name, 'Full CI build')) && '--minimal' || '' }}
-        run: ./generate.py ${GENERATE_CONFIG} ${GENERATE_MINIMAL} >>"${GITHUB_OUTPUT}"
+          # Configs flagged "extended" in the JSON are too expensive to run per
+          # pull request, so only the nightly schedule adds them. A manual run
+          # counts as well, because otherwise the only way to exercise one is to
+          # wait for the night.
+          GENERATE_EXTENDED: ${{ (github.event_name == 'schedule' || github.event_name == 'workflow_dispatch') && '--extended' || '' }}
+        run: ./generate.py ${GENERATE_CONFIG} ${GENERATE_MINIMAL} ${GENERATE_EXTENDED} >>"${GITHUB_OUTPUT}"

BUILD.md

@@ -273,7 +273,7 @@ Example use with some cmake variables set:
 ```
 cd .build
 conan install .. --output-folder . --build missing --settings build_type=Debug
-cmake -DCMAKE_BUILD_TYPE=Debug -Dcoverage=ON -Dxrpld=ON -Dtests=ON -Dcoverage_test_parallelism=2 -Dcoverage_format=html-details -Dcoverage_extra_args="--json coverage.json" -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake ..
+cmake -DCMAKE_BUILD_TYPE=Debug -Dcoverage=ON -Dxrpld=ON -Dtests=ON -Dcoverage_format=html-details -Dcoverage_extra_args="--json coverage.json" -DCMAKE_TOOLCHAIN_FILE:FILEPATH=build/generators/conan_toolchain.cmake ..
 cmake --build . --target coverage
 ```
 

cmake/XrplSanitizers.cmake

@@ -77,19 +77,24 @@ if(is_clang)
     message(STATUS "  Ignorelist: ${ignorelist_path}")
 endif()
 
-# Define SANITIZERS macro for BuildInfo.cpp
+# Define the SANITIZERS macro for BuildInfo.cpp, plus one of XRPL_ASAN,
+# XRPL_TSAN and XRPL_UBSAN per active sanitizer, so that code can test for a
+# specific one with #ifdef instead of parsing the dot-joined SANITIZERS string.
 set(sanitizers_list)
 if(SANITIZERS MATCHES "address")
     set(enable_asan ON)
     list(APPEND sanitizers_list "ASAN")
+    target_compile_definitions(common INTERFACE XRPL_ASAN)
 endif()
 if(SANITIZERS MATCHES "thread")
     set(enable_tsan ON)
     list(APPEND sanitizers_list "TSAN")
+    target_compile_definitions(common INTERFACE XRPL_TSAN)
 endif()
 if(SANITIZERS MATCHES "undefinedbehavior")
     set(enable_ubsan ON)
     list(APPEND sanitizers_list "UBSAN")
+    target_compile_definitions(common INTERFACE XRPL_UBSAN)
 endif()
 
 if(sanitizers_list)

docs/build/sanitizers.md

@@ -99,6 +99,40 @@ export TSAN_OPTIONS="include=sanitizers/suppressions/runtime-tsan-options.txt:su
 
 More details [here](https://github.com/google/sanitizers/wiki/ThreadSanitizerCppManual).
 
+> [!IMPORTANT]
+> The `ubuntu-clang-debug-amd64-tsan` CI config runs TSan in the full matrix
+> only, and reports nothing back. `runtime-tsan-options.txt` sets
+> `halt_on_error=false`, so the run continues past a finding, and the workflow
+> appends `exitcode=0` to `TSAN_OPTIONS`, so the finding does not fail the job.
+> Both are needed: TSan exits 66 on its own once it has reported anything. Read
+> the job log to see findings. A test that must fail on one has to run in its own
+> step with `halt_on_error=1` and `exitcode=66`.
+>
+> `exitcode=0` lives in the workflow and not in `runtime-tsan-options.txt`,
+> because the local command above reads that same file. A local run keeps the
+> default nonzero exit, so a finding fails the command. `exitcode=0` also only
+> covers TSan's own exit path: a test that fails on its own still fails the job.
+>
+> Reporting nothing back is a first stage, not the end state. Once the set of
+> findings the job produces is known and stable, drop `exitcode=0` from the
+> workflow so that a new finding fails the job, and suppress what is left in
+> third-party code.
+
+> [!IMPORTANT]
+> Run TSan on Linux to check lock order. Linux reports an inversion as
+> `WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock)`, with no
+> `TSAN_OPTIONS` needed. macOS arm64 reports nothing, not even a genuine double
+> lock of a non-recursive mutex, and not with `detect_deadlocks=1` or raw
+> `pthread_mutex_t` either. The report strings are present in its runtime and the
+> flag defaults to true, so that is a platform limit rather than a configuration
+> mistake. Data race detection does work on macOS.
+
+> [!TIP]
+> The build defines `XRPL_TSAN` when TSan is active, and `XRPL_ASAN` and
+> `XRPL_UBSAN` for the other two. Use them to skip a test that only means
+> something under one sanitizer. Skip at run time rather than with `#ifdef`
+> around the body, so that every build still compiles the test.
+
 ### LeakSanitizer (LSan)
 
 LSan is automatically enabled with ASAN. To disable it:
@@ -150,15 +184,39 @@ More details [here](https://clang.llvm.org/docs/undefinedbehaviorSanitizer.html)
 
 ### [`tsan.supp`](../../sanitizers/suppressions/tsan.supp)
 
-- **Purpose**: Suppress ThreadSanitizer data race warnings
-- **Format**: `race:<pattern>` where pattern matches function/file names
+- **Purpose**: Suppress ThreadSanitizer warnings
+- **Format**: `<type>:<pattern>` where pattern matches function/file names, and
+  type is `race`, `deadlock`, `signal`, `mutex` or `called_from_lib`
 - **More info**: [ThreadSanitizer suppressions](https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions)
+- **Note**: Every `deadlock:` pattern must name a source file. One that names a
+  locking primitive instead, such as `pthread_rwlock_rdlock`, turns lock-order
+  checking off for every lock of that kind in the tree, which for that example is
+  every `std::shared_mutex` read lock. Suppress the file that reports the
+  inversion instead. A pattern of any type that names a file which has since
+  moved matches nothing, so check the paths when a subsystem is relocated.
 
 ### [`sanitizer-ignorelist.txt`](../../sanitizers/suppressions/sanitizer-ignorelist.txt)
 
 - **Purpose**: Compile-time ignorelist for all sanitizers
 - **Usage**: Passed via `-fsanitize-ignorelist=absolute/path/to/sanitizer-ignorelist.txt`
-- **Format**: `<level>:<pattern>` (e.g., `src:Workers.cpp`)
+- **Format**: `<entity>:<glob>` (e.g. `src:*Workers.cpp`)
+- **Note**: This file is not a suppressions file, and the syntax differs. Clang
+  looks up only the entities `src`, `fun`, `global`, `type` and `mainfile`. A
+  `race:`, `deadlock:` or `signal:` entry parses without an error and is then
+  never consulted, so it does nothing; those types belong in `tsan.supp`.
+- **Note**: The glob must match the whole path as the compiler receives it, which
+  the build makes absolute. So `src:core/detail/Workers.cpp` matches nothing,
+  while `src:*core/detail/Workers.cpp` matches.
+- **Note**: To confirm that an entry works, compile the file with and without the
+  entry and compare how many times the object references the calls the sanitizer
+  plants at an instrumented operation: `__tsan_read` and `__tsan_write`,
+  `__asan_report`, or `__ubsan_handle`. For example
+  `nm -u <file>.o | grep -cE '__tsan_(read|write)'`. A working entry lowers the
+  count, usually to zero, though code inlined from a header still counts because
+  a `src:` glob matches the file a function is defined in. Do not look for
+  `__tsan_func_entry`, which survives an ignorelist, or `__tsan_init`, which only
+  shows the runtime is present; either one reports an ignored file as
+  instrumented.
 
 ## Troubleshooting
 

sanitizers/suppressions/runtime-tsan-options.txt

@@ -1,3 +1,4 @@
 halt_on_error=false
 verbosity=1
+detect_deadlocks=1
 second_deadlock_stack=1

sanitizers/suppressions/sanitizer-ignorelist.txt

@@ -1,32 +1,23 @@
-# We were seeing some false positives and some repeated errors(since these are library files) in following files.
-# Clang will skip instrumenting the files added here.
-# We should fix the underlying issues(if any) and remove these entries.
-
-deadlock:libxrpl/beast/utility/beast_Journal.cpp
-deadlock:libxrpl/beast/utility/beast_PropertyStream.cpp
-deadlock:test/beast/beast_PropertyStream_test.cpp
-deadlock:xrpld/core/detail/Workers.cpp
-deadlock:xrpld/core/JobQueue.cpp
-
-race:libxrpl/beast/utility/beast_Journal.cpp
-race:libxrpl/beast/utility/beast_PropertyStream.cpp
-race:test/beast/beast_PropertyStream_test.cpp
-race:xrpld/core/detail/Workers.cpp
-race:xrpld/core/JobQueue.cpp
-
-signal:libxrpl/beast/utility/beast_Journal.cpp
-signal:libxrpl/beast/utility/beast_PropertyStream.cpp
-signal:test/beast/beast_PropertyStream_test.cpp
-signal:xrpld/core/detail/Workers.cpp
-signal:xrpld/core/JobQueue.cpp
-
-src:beast/utility/beast_Journal.cpp
-src:beast/utility/beast_PropertyStream.cpp
-src:core/detail/Workers.cpp
-src:core/JobQueue.cpp
-src:libxrpl/beast/utility/beast_Journal.cpp
-src:test/beast/beast_PropertyStream_test.cpp
-src:src/test/app/Invariants_test.cpp
+# Clang skips instrumenting the files listed here. Keep the list as short as it
+# can be: an entry here removes a file from every sanitizer's view, so it hides
+# real findings along with the false positive it was added for.
+#
+# This is a clang ignorelist, not a runtime suppressions file, and the two take
+# different syntax. Here an entry reads <entity>:<glob>, and the only entities
+# clang looks up are src, fun, global, type and mainfile. A `race:`, `deadlock:`
+# or `signal:` entry parses without an error and is then never consulted, so it
+# does nothing; those types belong in tsan.supp. The glob has to match the whole
+# path as the compiler receives it, which the build makes absolute, so an entry
+# needs a leading `*` to match anything at all. See docs/build/sanitizers.md.
+#
+# This list once held seven `src:` entries for beast_Journal.cpp,
+# beast_PropertyStream.cpp, Workers.cpp, JobQueue.cpp, the PropertyStream test
+# and Invariants_test.cpp, added against false positives. None carried the
+# leading `*`, so none ever matched, and every one of those files has been
+# instrumented the whole time. The sanitizer jobs pass that way, so the
+# suppressions are not needed; they are removed rather than repaired. Restore one
+# only with a finding that shows it is needed, and check it works by the method
+# in sanitizers.md.
 
 # ASan false positive: stack-use-after-scope in ErrorCodes.h inline functions.
 # When Clang inlines the StaticString overloads (e.g. invalidFieldError(StaticString)),

sanitizers/suppressions/tsan.supp

@@ -35,43 +35,52 @@ race:src/xrpld/app/main/BasicApp.cpp
 race:src/xrpld/app/main/GRPCServer.cpp
 race:src/xrpld/app/misc/detail/AmendmentTable.cpp
 race:src/xrpld/app/misc/FeeVoteImpl.cpp
-race:src/xrpld/app/rdb/detail/Wallet.cpp
+race:src/libxrpl/server/Wallet.cpp
 race:src/xrpld/overlay/detail/OverlayImpl.cpp
-race:src/xrpld/peerfinder/detail/PeerfinderManager.cpp
-race:src/xrpld/peerfinder/detail/SourceStrings.cpp
+race:src/libxrpl/peerfinder/PeerfinderManager.cpp
+race:src/libxrpl/peerfinder/SourceStrings.cpp
 race:src/xrpld/rpc/detail/ServerHandler.cpp
 race:xrpl/server/detail/Door.h
 race:xrpl/server/detail/Spawn.h
 race:xrpl/server/detail/ServerImpl.h
 race:xrpl/nodestore/detail/DatabaseNodeImp.h
 race:src/libxrpl/beast/utility/beast_Journal.cpp
-race:src/test/beast/LexicalCast_test.cpp
+race:src/tests/libxrpl/beast/LexicalCast.cpp
 race:ServerHandler
 
 # More suppressions in external library code.
 race:crtstuff.c
 race:pipe
 
-# Deadlock / lock-order-inversion suppressions
-# Note: GCC's TSAN may not fully support all deadlock suppression patterns
-deadlock:src/libxrpl/beast/utility/beast_Journal.cpp
-deadlock:src/libxrpl/beast/utility/beast_PropertyStream.cpp
-deadlock:src/test/beast/beast_PropertyStream_test.cpp
-deadlock:src/xrpld/core/detail/Workers.cpp
-deadlock:src/xrpld/app/misc/detail/Manifest.cpp
-deadlock:src/xrpld/app/misc/detail/ValidatorList.cpp
-deadlock:src/xrpld/app/misc/detail/ValidatorSite.cpp
+# Deadlock / lock-order-inversion suppressions: deliberately none.
+#
+# The TSan job exists to find lock-order inversions, and it cannot fail a build,
+# so a suppression here costs a finding and buys nothing. Ten used to sit below:
+#
+#   - pthread_create, pthread_rwlock_rdlock and boost::asio, which name locking
+#     primitives rather than source files. Each of those turns lock-order
+#     checking off for every lock of that kind in the tree, which for
+#     pthread_rwlock_rdlock is every std::shared_mutex read lock, and between
+#     them they covered any inversion reached through a strand or a thread start.
+#   - ValidatorList.cpp and ValidatorSite.cpp.
+#   - beast_Journal.cpp, beast_PropertyStream.cpp, the PropertyStream test,
+#     Workers.cpp and Manifest.cpp.
+#
+# Manifest.cpp was the expensive one. ManifestCache::save() invokes the caller's
+# predicate while holding its own lock, and the validator list takes the pair the
+# other way, so the two deadlock. Removing that one line surfaced the cycle 52
+# times in a single run, on the shutdown path.
+#
+# If one has to come back, it must name a source file, and check that the file is
+# where the pattern says: a suppression naming a moved file protects nothing.
+# Note also that GCC's TSAN may not fully support all deadlock suppression
+# patterns.
 
 signal:src/libxrpl/beast/utility/beast_Journal.cpp
-signal:src/xrpld/core/detail/Workers.cpp
-signal:src/xrpld/core/JobQueue.cpp
+signal:src/libxrpl/core/detail/Workers.cpp
+signal:src/libxrpl/core/detail/JobQueue.cpp
 signal:Workers::Worker
 
-# Aggressive suppressing of deadlock tsan errors
-deadlock:pthread_create
-deadlock:pthread_rwlock_rdlock
-deadlock:boost::asio
-
 # Suppress SEGV crashes in TSAN itself during stringbuf operations
 # This appears to be a GCC-15 TSAN instrumentation issue with basic_stringbuf::str()
 # Commonly triggered in beast::Journal::ScopedStream destructor