A reachable shard panic is never repaired, so the shard silently returns zero results indefinitely

[aaaaaandrew]

Environment

zoekt at 2cb19912a4073e5a9895658b7cb135ee4b35733b (main), zoekt-webserver,
linux/arm64, Go 1.26.

Summary

Two independent facts combine into silent, unbounded data loss:

1. A malformed shard panics inside the search path (§2, §3).
2. A recovered per-shard panic is never repaired, and the shard is never
   re-opened (§1).

The result is that one bad shard drops out of every subsequent query while the
API keeps returning HTTP 200 with a plausible-looking 0 match count. Clients
that don't inspect Stats.Crashes report a confident false negative. All three
findings below reproduce on unmodified zoekt at the SHA above.

1. A recovered shard panic is never repaired

searchOneShard recovers the panic, sets Stats.Crashes = 1, and drops that
shard's results. That contains the blast radius, but nothing re-opens the shard:

1. searchOneShard — the deferred recover() sets sr.Stats.Crashes = 1 and
   returns an empty result. The shard stays in the searcher set, unchanged.
2. DirectoryWatcher.scan (search/watcher.go) builds ts[fn] from Lstat
   mtimes and loads only where s.timestamps[k] != mtime. A file whose mtime has
   not changed is never in toLoad.
3. Nothing else triggers a reload. loadShard and shardedSearcher.replace do
   exactly the right thing, but nothing calls them in response to a panic.

So the shard stays broken until an unrelated re-index happens to touch that
repository. It is also invisible to load metrics — the shard loaded successfully
and never failed to load, so zoekt_shards_load_failed_total stays at zero.
listOneShard has the same recovering defer and the same gap.

2. newCompressedPostingIterator slices with an unchecked negative length

binary.Uvarint returns negative n on overflow (a varint longer than 64 bits),
where -n is the number of bytes read. newCompressedPostingIterator
(index/hititer.go) uses it unchecked:

d, sz := binary.Uvarint(b)
return &compressedPostingIterator{
    _first:           uint32(d),
    blob:             b[sz:],        // sz is negative on overflow
    indexBytesLoaded: sz,
    what:             w,
}

Calling it with a malformed posting list panics:

// 11 continuation bytes => Uvarint reports overflow with n = -11
blob := []byte{0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x01}
newCompressedPostingIterator(blob, ngram(0))
// panic: runtime error: slice bounds out of range [-11:]

indexBytesLoaded also goes negative, so any stat derived from it is wrong.

3. Corrupt shard bytes panic and hang in the live search path

The unchecked reads are reachable from a real search, not just a direct
constructor call. Building a shard with index.NewBuilder, overwriting a mid-file
window with 0x80 bytes, opening it with NewSearcher and running Search() for
a term that exists:

• NewSearcher accepts the corrupted shard — the damage is not caught at
  load.
• The search then panics: runtime error: index out of range [7] with length 0.
• Other corruptions of the same shape make Search() hang, blowing a
  two-minute test timeout rather than returning.

The hang is the more serious of the two: recover() cannot catch it, so the
per-shard containment in §1 does not apply and the query occupies its worker until
the caller's own deadline fires. A deployment without a client-side search
deadline has no bound at all.

Suggested direction

• Bounds-check the varint length in newCompressedPostingIterator (and audit the
  sibling readers) so malformed input yields an error rather than a panic.
• Give the recover() in searchOneShard/listOneShard a repair path: schedule a
  re-open of that shard (loadShard + replace) so the next query gets a fresh
  descriptor and mapping. Worth noting if you take this route: the swap needs to be
  conditional on the faulted shard still being the installed one, since the watcher
  runs concurrently and may have installed a newer generation or dropped the key —
  and the losing load needs closing or its fd and mapping leak. The faulted
  response should keep its Stats.Crashes; rewriting it as complete reintroduces
  the same false negative from the other side.

Field note: the same silent zero from a dead mmap

We reached §1 from a different direction, which may help anyone debugging similar
symptoms. Shards mapped MAP_SHARED from network storage produced:

panic: runtime error: invalid memory address or nil pointer dereference
  encoding/binary.Uvarint(...)
  index.newCompressedPostingIterator   index/hititer.go
  index.(*indexData).trigramHitIterator index/hititer.go
  → recovered by search.searchOneShard.func1

This reads as shard corruption and is not. readSectionBlob returned err == nil
with a non-empty slice — the offsets were in bounds — yet the slice's data
pointer was non-canonical (0xfff…, above the 47-bit user VA) while sibling
pointers were normal. Wrong pointer, not wrong bytes: the mapping's backing
object had gone away, and with MAP_SHARED the kernel keeps the mapping present so
bounds checks still pass while reads return garbage. Reproducible standalone: mmap
a file MAP_SHARED, truncate it so the pages lose their backing store, then read
it. Integrity checks confirmed the file itself was intact; only the live mapping
was dead, and re-opening the shard fixed it immediately — which is what pointed at
§1.

[linear]

CU-3185