skip to content

A report-export handler shares one json.Encoder across goroutines and the download is corrupt. Why?

level: seniorimportance: should knowfreq 38%

answer

  1. the type has state, and it is shared
  2. one Write is not one atomic record
  3. the status code has already left
  4. give the writer a single owner

basics

~20 s

A json.Encoder has no internal locking, and neither does an http.ResponseWriter. Concurrent Encode calls race on shared state and their writes interleave, splicing records together. Give one goroutine the encoder and feed it rows over a channel.

solid answer

~50 s

`json.Encoder` is not safe for concurrent use: it holds a writer, an indent setting and a sticky error, and nothing synchronises them. Neither is `http.ResponseWriter`. Each `Encode` currently assembles a value and hands it over in one `Write`, but that is an implementation detail, and it says nothing about the writer underneath — a shared `bufio.Writer` can interleave mid-record. The result is a download with spliced JSON that only reproduces under load. Run the export path under `-race` and the detector names the conflicting accesses. The fix is ownership, not more locking: producers send rows on a channel, one goroutine ranges over it and owns the encoder and the flush. A mutex around every `Encode` also works but serialises anyway. Note too that once the first row is written the 200 has been sent, so a later failure cannot become a 500.

code

go · 14 lines
go
// w is the http.ResponseWriter; several workers send into rows.
rows := make(chan Row)
go produceRows(ctx, rows) // closes rows when done

enc := json.NewEncoder(w)
rc := http.NewResponseController(w)
for row := range rows {
	if err := enc.Encode(row); err != nil {
		return err // client gone, or write failed: stop now
	}
	if err := rc.Flush(); err != nil {
		return err
	}
}

go deeper

for a junior

Know that most Go types are not safe for concurrent use unless the documentation says so, and that a json.Encoder and an http.ResponseWriter are both in that category.

for a middle

Explain what is shared: the encoder's writer, indent settings and recorded error, plus the response writer underneath, and why buffered writes can splice two records together rather than merely reorder them.

for a senior

Demonstrate the whole loop — reproduce under the race detector, restructure to a single writing goroutine fed by a channel, handle a failure that arrives after the status has been sent, and decide where to flush.

for a principal

Own the streaming contract across services: how a consumer distinguishes a complete export from a truncated one, whether partial output is ever acceptable, and who absorbs the cost when a long download fails halfway.

## What is actually shared A `*json.Encoder` is a small struct with mutable state: the `io.Writer` it wraps, the indent prefix and unit set by `SetIndent`, the `SetEscapeHTML` flag, an internal indent buffer, and a recorded error. `Encode` reads and writes several of those. Two goroutines calling `Encode` on the same encoder are two goroutines mutating the same struct with no synchronisation, which is a data race in the precise sense: the memory model gives you no guarantee about what either goroutine observes, and the program is free to misbehave in ways that are not simply "two records in the wrong order". Underneath sits a second unsafe object. An `http.ResponseWriter` is documented as not safe for concurrent use, and a `bufio.Writer` is not either. So even if the encoder itself were locked, concurrent writes to the response would still be a race. ## Why "each Encode does one Write" is not a defence A popular rationalisation is that `Encode` marshals into an internal buffer and issues a single `Write`, so records cannot interleave. That is what current implementations do, but it is not part of the API contract, and it does not make the surrounding state accesses safe. More importantly, a single `Write` call on the *encoder's* writer is not an atomic append at the socket if that writer is itself buffered: two `Write` calls into one `bufio.Writer` from two goroutines can leave a half-copied record in the buffer. What arrives at the client is a line that is the front of one record and the tail of another — invalid JSON that the consumer reports as a parse error at a byte offset the server has no record of. ## Confirming it rather than guessing The failure is load-dependent and will not reproduce in a single-request test, which is exactly the shape of bug the race detector exists for. Build or test the export path with `-race` and drive genuine concurrency through it — several producers, enough rows to overlap. The detector reports the conflicting reads and writes with both stacks, naming the encoder and the response writer. Its limitation matters here: it only finds races on paths actually executed, so a clean run of a test that never overlaps two producers proves nothing. Pair it with the corrupted output you already have as evidence. ## The fix: single ownership Fan out the work, fan in the writing. Producers compute rows and send them on a channel; exactly one goroutine ranges over that channel, calls `Encode`, and decides when to flush. That goroutine owns the encoder and the response writer for the life of the handler, and no synchronisation primitive is needed beyond the channel itself. It also gives you one place to handle a client that disconnects, one place to observe the encoder's sticky error, and a natural point to apply backpressure — an unbuffered or small-buffered channel makes slow clients slow the producers instead of accumulating rows in memory. A `sync.Mutex` held across each `Encode` call is a correct alternative and is sometimes the smaller diff. But it serialises the writes anyway, so it buys no concurrency, and it spreads the error handling across every producer. Reach for it when you cannot restructure; prefer ownership when you can. What is definitely wrong is giving each goroutine its own encoder over the same writer, or its own `bufio.Writer` over the response. That changes nothing about the shared destination and usually makes the interleaving worse. ## The status code is already gone The second half of this problem is protocol-shaped. As soon as the first row is written, the response header has been sent with a 200. If row nine thousand fails, you cannot retract it — calling `http.Error` at that point appends an error message to a body the client is already parsing as records. The workable options are to write a terminal record that the client is required to see (so a stream without it is treated as failed), to use an HTTP trailer, and to log the failure server-side with the offset reached. Whichever you choose, the client contract must say that a truncated stream is a failure; a consumer that treats "the connection ended" as "the export finished" will silently ingest partial data. For progressive delivery, flush after each row or each batch: the encoder does not flush, and buffered bytes sit until the buffer fills or the handler returns. `http.NewResponseController(w).Flush()` is the modern form; the older `w.(http.Flusher)` type assertion does the same thing. Flushing per row costs syscalls, so batching the flush is the usual compromise once you have measured it with a benchmark run under `-benchmem`, which is also where you see the allocation win from encoding per row instead of building the whole export in memory first.

  • Would a mutex held around every Encode call fix it?
    It removes the race and the interleaving, so yes, correctness is restored. But the writes were going to serialise regardless, so it buys no throughput, and it scatters the error handling and flushing across every producer. A single writer goroutine draining a channel is usually the clearer design and gives you a natural backpressure point.
  • The corruption only appears under load. How do you demonstrate it is a data race?
    Run the export path under `-race` with a test that drives real concurrency — several producers and enough rows to overlap. The detector prints both conflicting stacks, naming the encoder and the response writer. Remember it only catches races on code paths actually executed, so a clean run of a single-producer test proves nothing.
  • Row nine thousand fails after the download has started. What can the handler still do?
    Not change the status: the 200 and the earlier rows are already at the client. Write a terminal error record, or an HTTP trailer, so the consumer can tell a completed stream from a truncated one, and log the failure with the row or byte offset reached. The client contract has to treat a stream missing its terminator as failed.
  • How would you show that per-row encoding is cheaper than building the whole export first?
    Benchmark both against the same data with `-benchmem`. The per-row version shows markedly lower bytes per operation and allocations per operation because it never materialises one large encoded buffer, while output the client receives is unchanged — which is precisely the evidence to bring when the goal is speed without a behaviour change.

saying these in an interview costs you the question

  • Says json.Encoder is concurrency-safe because Encode does one Write
  • Gives each goroutine its own encoder over the same writer
  • Proposes returning a 500 after the body has started
  • Treats one clean race-detector run as proof of correctness
  • Blames the client or the network for the spliced records