tests: add thread_tests for concurrent context use, run under Helgrind in CI #1941

pull erickcestari wants to merge 3 commits into bitcoin-core:master from erickcestari:thread-tests changing 9 files +488 −1
  1. erickcestari commented at 1:39 AM on September 24, 2026: none

    include/secp256k1.h says a constructed context can be used from multiple threads simultaneously, but nothing in the repo tests this. The unit test runner parallelizes with fork(), so no test ever runs threads.

    This adds thread_tests, which starts 4 threads on one randomized context. Each thread clones the context and then calls the API of every enabled module: ECDSA, ECDH, recovery, extrakeys, schnorrsig, musig, ellswift and silentpayments. Verification calls use secp256k1_context_static, since callers share that between threads too.

    The test is meant to run under a race detector such as Helgrind or ThreadSanitizer, which reports any write to shared state from a call that takes a const context.

    Build:

    • New options --enable-thread-tests and SECP256K1_BUILD_THREAD_TESTS. They default to on when tests are enabled and pthreads are available. Configure fails if they are requested explicitly and pthreads are missing.
    • Only thread_tests links against pthreads. The library does not.
    • On MinGW the test links statically, so it doesn't need the winpthreads DLL at runtime. Without that it fails under wine. MSVC has no pthreads, so the test is off there. (I haven't tested it on Windows).

    CI:

    • ci.sh jobs pass --enable-thread-tests=yes.
    • Where Valgrind is enabled, ci.sh also runs thread_tests under Helgrind, next to the existing Valgrind run of ctime_tests. That covers x86_64, i686 and arm64 Linux without a new job.
    • The ASan job now sets WITH_VALGRIND: 'no', since Valgrind can't run ASan binaries.

    Follow-ups:

    These are left out to keep this PR small:

    • Run the test with a custom SHA256 compression function set through secp256k1_context_set_sha256_compression. Apart from the callbacks, that's the only runtime state a context holds, and the test currently only uses the default.
    • Add a TSan job if the library ever uses atomics, for example for lazy initialization. Helgrind and DRD report false positives on atomics that aren't annotated, and TSan handles them correctly.
    • Test the read-write lock pattern that secp256k1.h recommends for re-randomizing a shared context: one thread calls secp256k1_context_randomize under the write lock while the others sign under read locks.
    • Call the remaining 16 const-context functions. They are all parse, serialize or convert helpers, such as secp256k1_ecdsa_signature_parse_compact and the musig nonce and partial signature (de)serializers.
    • Exercise error paths by passing invalid inputs, such as an out-of-range secret key or a bad pubkey encoding, so that failing calls also run concurrently.

    | I wrote this with help from Claude Code, and I've gone through all of it myself.

  2. tests: add thread_tests
    secp256k1.h documents that a context can be used from multiple threads
    simultaneously, but nothing tested it. Meant to run under a race detector
    such as Helgrind or ThreadSanitizer.
    024d065792
  3. build: add thread-tests option
    Built by default when tests are enabled and pthreads are available.
    50ba766c2f
  4. ci: run thread_tests in all jobs and under Helgrind b4e9cb6d6f
  5. erickcestari force-pushed on Sep 24, 2026
  6. real-or-random added the label assurance on Sep 24, 2026
  7. real-or-random added the label feature on Sep 24, 2026
Contributors

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-09-25 04:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me