skip to content

A log tailer makes map keys with unsafe.String over bufio.Scanner.Bytes(); the counts go wrong once more lines arrive. Why?

level: seniorimportance: should knowfreq 35%

answer

  1. ask who owns those bytes
  2. the next read writes over them
  3. the hash was taken from the old content
  4. a test that reads one more line

basics

~20 s

bufio.Scanner.Bytes() returns a slice into the scanner's own buffer, which the next Scan overwrites. The zero-copy string borrows those bytes, so every retained map key silently changes content while the map still files it under the old hash.

solid answer

~50 s

The scanner reads into one internal buffer and reuses it; `Bytes()` is documented as valid only until the next call to `Scan`. Building the key with `unsafe.String(unsafe.SliceData(line), len(line))` borrows those bytes instead of copying them, so the map ends up holding keys whose content is rewritten by the very next line. Since a map hashes a key once, at insert, the entries are stranded: lookups miss, the same logical line inserts again and again, and nothing can delete the stale entries. No tool reports it — it is one goroutine writing its own buffer, so the race detector is silent, and the unit test passes because it asserts before the next `Scan`. The fix is a copying conversion at the boundary — `string(line)` or `strings.Clone` — keeping the zero-copy form only for values consumed inside the iteration.

code

go · 5 lines
go
for sc.Scan() {
	line := sc.Bytes() // valid only until the next Scan
	key := unsafe.String(unsafe.SliceData(line), len(line))
	counts[key]++ // the stored key's bytes change next iteration
}

go deeper

for a junior

Remember the ownership question: a slice handed to you by a reader usually points into that reader's own buffer, so keeping it past the next read keeps a view of bytes that have already been replaced.

for a middle

Explain the two mechanics that combine here: Bytes returns a view into a reused buffer, and a map hashes a key once at insert, so mutated key bytes leave the entry stranded under a stale hash.

for a senior

Demonstrate the diagnosis and the guard. Say why the race detector is silent, why the existing test passes, and how a test that scans an extra line before asserting turns the bug deterministic before you touch the fix.

for a principal

Own the interface rule. Decide whether borrowed strings may cross a package boundary at all, and require any function that returns one to say so in its first doc line rather than relying on reviewers to remember.

## What the scanner promises `bufio.Scanner` reads into a buffer it owns and hands you a view of the current token. `Bytes()` returns a slice pointing into that buffer, and its documentation says the underlying array may be overwritten by a subsequent call to `Scan`. `Text()` exists as the copying alternative: it allocates a fresh string for the token, which is why it is safe to keep and why it costs an allocation per line. So the buffer is a moving window over the file. Whatever you hold that points into it is only meaningful until the next line is read. ## What the zero-copy conversion changes `unsafe.String(unsafe.SliceData(line), len(line))` produces a string over that same array. Nothing is copied, which is the entire point on a hot ingest path — the copy per line is real and measurable. But the result inherits the scanner's lifetime rule, and the rule is now invisible: a `string` value carries no hint that its bytes are borrowed. When that string is put into a map as a key, the map keeps it. On the next iteration the scanner writes the next line into the same array, and the key's bytes change underneath the map. ## Why the map is destroyed rather than merely wrong A Go map hashes a key once, when the entry is inserted, and places it in the bucket that hash selects. It never revisits or rehashes stored keys. After the buffer is refilled: - A lookup for the line's real text hashes the correct content and probes a bucket that does not hold the entry, so it misses — and the loop inserts a second entry for the same logical line. - A lookup for whatever the bytes now say hashes to the entry's bucket only by accident, and even then the stored key's bytes have moved on again. - `delete` fails for the same reason from both directions, so the stale entries are permanent. They still count toward `len(m)` and still appear in iteration. The symptom reported by whoever is on call is "the counts are wrong and the map keeps growing", which points at the aggregation logic rather than at a conversion three functions away. ## Why nothing catches it There is a single goroutine here. It writes to a buffer it owns and reads it back in order, so there is no data race and a `-race` build has nothing to report. `go vet` does not model borrowed bytes. The compiler is content: the types are correct. Worse, the obvious unit test passes. Feed one line, assert the count, done — the borrowed bytes are still intact because nothing refilled the buffer. The test that finds this feeds at least two lines and asserts *after* the loop, or scans one more line before checking the stored key. That single change turns an intermittent production mystery into a deterministic failure, and it is the regression test worth keeping. ## Fixing it The fix is to copy at the ownership boundary and only there: - Anything retained past the iteration gets a real copy: `key := string(line)`, or `strings.Clone(s)` if you already have a borrowed string in hand. - Anything consumed inside the iteration may keep borrowing: hashing it for a lookup, comparing it against a prefix, parsing a number out of it, matching it against a set. These die before the next `Scan`. Note that `m[string(line)]` as a *lookup* does not even allocate — the compiler removes that copy — so the zero-copy conversion buys nothing there. The allocation only appears where the string is retained, which is precisely where it is required. A second, blunter fix is to give the retained value its own storage: append the bytes onto an arena you control, and convert a stable region of it. That trades one allocation per line for a growth strategy you must then manage, and it is only worth it if a benchmark with `-benchmem` says the per-line copy actually dominates. `go build -gcflags=-m` will show you which conversion escapes to the heap so you can confirm the copy you removed is the one you meant. ## The part that outlives the fix If a helper returns a borrowed string at all, its doc comment must say so in the first line: the returned string aliases a reused buffer and is only valid until the next read, and callers who keep it must copy it. Better still, make the safe form the default and expose the borrowing variant under a name that cannot be used by accident. A lifetime rule that lives only in a reviewer's memory will be violated by the next person who reads the signature and sees a perfectly ordinary `string`.

  • Why does the unit test pass when the bug is real?
    Because it scans one line, asserts and stops: nothing has refilled the buffer, so the borrowed bytes are still the line you inserted. Feed at least two lines and assert after the loop — or scan once more before checking the stored key — and the corruption becomes deterministic. That is the regression test to keep, since it fails for the right reason without needing timing or concurrency.
  • Where in that loop can the zero-copy conversion legitimately stay?
    Anywhere the string dies before the next `Scan`: hashing it for a map lookup, comparing it against a prefix or a set, parsing a number out of it, matching it with a switch. It is a lifetime rule, not a size rule — borrow for the duration of the iteration and copy anything that outlives it. Note that a map lookup with a converted key does not allocate anyway, so borrowing buys nothing there.
  • What belongs in the doc comment of a function that returns such a borrowed string?
    State the aliasing and the expiry in the first sentence: the returned string shares the reader's buffer and is only valid until the next read, so a caller that retains it must copy it first. A plain `string` return type gives the caller no other warning. Where you can, make the copying version the default and give the borrowing one a name that signals the hazard.
  • Would the race detector help here?
    No. One goroutine reads the file, rewrites its own buffer and updates the map in order, so there is no unsynchronised concurrent access to report. This is a lifetime violation, not a race. If the same borrowed strings were also handed to worker goroutines you would eventually get race reports, but they would point at the sharing rather than at the borrowing that caused it.

It is like handing out business cards that are really one whiteboard you keep wiping and rewriting. Everyone still has a card; none of them says what it said.

saying these in an interview costs you the question

  • Blames a data race and adds a mutex
  • Says the map implementation is broken
  • Thinks Scanner.Bytes returns fresh storage each call
  • Copies the slice header and believes the bytes are safe
  • Fixes it by enlarging the scanner's buffer