Fix segfault in readNormalIncomplete when a reference loads before load() returns - #281

Thank you for maintaining ncbi-vdb. This fixes the segfault described in #280, where a small external reference finishes background loading before load() returns.

Problem

load() in libs/axf/refseq.c starts the background loading thread and only then sets result->reader = readNormalIncomplete. For a small reference, the thread can finish first: it sets reader = readNormal and async = NULL, and then load() puts readNormalIncomplete back. The next RefSeq_getBases() sees async == NULL and takes the fast path into readNormalIncomplete(), which dereferences the NULL async (async->rr.first, offset 0x18). This crashes fastq-dump 3.4.1 on ERR14844919. Details and stacks are in the issue.

Changes

Two commits, so the first can be taken on its own:

1. set RefSeq reader before starting the background loading thread. This moves the readNormalIncomplete assignment ahead of KThreadMake(). pthread_create() orders it before anything the thread does, so the thread's final reader always wins. This commit fixes the crash.
2. publish the end of RefSeq background loading with release/acquire ordering. runLoadThread() clears async with a release store, and RefSeq_getBases() reads it with acquire loads. A reader that sees async == NULL is then guaranteed to also see the final reader and the loaded bases. Plain volatile does not guarantee that on arm64. The change uses __atomic_* under __GNUC__ (gcc and clang), following the __GNUC__/_MSC_VER split in libs/klib/hashfile.c. Other compilers keep the volatile access; on MSVC x64 that already has acquire/release semantics (/volatile:ms, the default). The fast path stays lock-free. On x86-64, acquire loads and release stores compile to plain movs; on arm64 they become ldar/stlr.

Testing

On macOS 26.6.1 arm64, Apple clang, ./configure --with-debug && make all:

• The build succeeds, with no new warnings in libs/axf.
• Test_Axf_RestoreRead passes. It is the only test that exercises refseq.c, and it reads NC_000001.11 remotely.
• A deterministic reproduction, using a pthread_create shim that waits for the new thread to finish (up to 1 s) before returning, and a harness that calls RestoreReadGetSequence() on the 205 RefSeq-format references of ERR14844919:
  • engineering: assert(async != NULL) in positionToRow on the first reference, 5 of 5 runs.
  • This branch: all 205 references read, 5 of 5 runs.
  • Commit 1 alone: 3 of 3 clean. Commit 2 alone: 3 of 3 crash.
• Natural reproduction, with the same change applied to 3.3.0 (single-threaded reading of every reference of ERR14844919 under CPU contention): 9 of 12 runs crashed before the change, and 18 of 18 ran clean after it.

No regression test is included. Hitting the window reliably needs a hook such as the pthread_create shim above, which doesn't fit the existing test framework. The shim and harness are in the issue, and I'm happy to turn them into a test if you can suggest a place for one.

Not tested on Linux, Windows or x86-64. Commit 1 is platform-independent. On Windows, commit 2 relies on MSVC's default /volatile:ms for x64.

I dedicate these changes to the public domain, consistent with the project's Public Domain Notice. Thanks for considering them; I'm glad to rework the change in whatever way you prefer.