Lock globals in dump(), and handle a nil context

[youdie006]

Two problems in dumpAll, three lines apart

1. globals is read without the lock that guards it

Set.globals is mutex-protected everywhere else:

• set.go:218-222 — AddGlobal writes under gmx.Lock()
• set.go:226-231 — LookupGlobal reads under gmx.RLock()
• eval.go:201-203 — the template execution path reads under gmx.RLock()

and set.go:21 names the field's purpose: gmx *sync.RWMutex // global variables map mutex.

dump.go:26 does not:

vars = a.runtime.set.globals
for i, name := range vars.SortedKeys() {
	...
	val := vars[name]

So rendering a template containing {{ dump() }} while another goroutine calls AddGlobal races on the map — at the range and again at the index:

WARNING: DATA RACE
Write at 0x00c0000e8150 by goroutine 10:
  jet.(*Set).AddGlobal()   set.go:220

Previous read at 0x00c0000e8150 by goroutine 12:
  jet.VarMap.SortedKeys()  exec.go:31
  jet.dumpAll()            dump.go:27
  jet.(*Template).Execute()

A second report in the same run points at dump.go:31 (val := vars[name]). It reproduced on every run I tried.

Note dump("x") is already safe — that path goes through rnt.resolve, which takes the RLock. Only the no-argument and depth forms bypass it.

2. dump() panics on a nil context

Runtime.context is only assigned when data != nil (exec.go:66-68), so Execute(w, vars, nil) leaves it a zero reflect.Value and dump.go:19 calls .Type() on it:

reflect: call of reflect.Value.Type on zero Value

Deterministic, no concurrency needed. This is also what masked the race at first — the panic short-circuits execution before line 26 is reached.

The change

Copy the map under RLock and format outside it, so the lock is held only for the copy rather than for the whole formatting pass. And guard the context read with IsValid(), printing <nil> when there is no context.

Tests

TestDumpGlobalsWhileAddGlobal runs AddGlobal and Execute concurrently; TestDumpWithNilContext covers the nil-data path.

Reverting only dump.go and keeping the tests gives the race report back under -race, and the nil test fails with the reflect.Value.Type panic.

go test -race ./... is green across all packages with the change. The existing suite is already clean under -race — neither of these paths is covered by it, and CI does not pass -race (.travis.yml and appveyor.yml both run plain go test -v ./...), which is why they have stayed hidden.

One note: gofmt flags dump.go at HEAD over the doc-comment bullet indentation (the Go 1.19 comment reformat). I left that alone rather than mixing an unrelated reformat into this diff.

---

Disclosure: this patch was prepared with AI assistance. The race reproduction, the red/green check and the suite runs above were executed against this branch; happy to adjust anything on request.