Skip to content
All writing

Why Go crashed only in go test: uintptr and KeepAlive

A Windows API call was reading a buffer Go's garbage collector had already freed. The bug was invisible for weeks because it needs a GC cycle to land inside a window a few microseconds wide.

Go is the first compiled, garbage-collected language I've written anything real in. I came to it from JavaScript and PHP, where memory is simply not your problem, and I picked it for Devpit because I wanted one Windows executable with no runtime to install.

For a while the only hard thing about Go was that nothing was hard. Then a test run crashed. Not failed. Crashed, with a Go runtime fatal error, in a package whose own tests had passed minutes earlier.

It happened once. Running that package alone was green, every time. It took go test ./... across all 39 packages to produce it, and it has not produced it again since.

What the code was doing

Devpit sends some folders to the Recycle Bin instead of deleting them outright, which on Windows means calling SHFileOperationW. You fill in a struct, point one of its fields at a double-null-terminated UTF-16 path, and hand the struct to the API.

I was using a small third-party package for this. Its code looked like mine would have:

buf, _ := windows.UTF16FromString(path)
buf = append(buf, 0)
 
op := &shFileOpStruct{
    wFunc:  FO_DELETE,
    pFrom:  uintptr(unsafe.Pointer(&buf[0])),
    fFlags: FOF_ALLOWUNDO,
}
ret, _, _ := shFileOperationW.Call(uintptr(unsafe.Pointer(op)))

That reads fine. buf is a live slice, its address goes into the struct, the struct goes to Windows. Nothing is freed.

Except buf is dead one line later, and Go is allowed to act on that.

The rule I didn't know

unsafe.Pointer is a pointer the garbage collector understands. uintptr is just a number.

The moment you write uintptr(unsafe.Pointer(&buf[0])) and store the result somewhere, the collector stops seeing any connection between that value and buf. It is an integer that happens to look like an address.

So the compiler asks the only question it can: is buf used after this line? It isn't. The last statement mentioning buf is the one that took its address. The backing array becomes collectable right there, while Windows is still holding the number that points at it.

I assumed passing the struct to Call would cover it. It doesn't. Go has a special rule (//go:uintptrkeepalive) that keeps direct syscall arguments alive across the call, and it does apply here, to op. It does not follow op's own uintptr fields to whatever they point at, because as far as the compiler is concerned those are numbers, not pointers.

The fix is one line:

ret, _, _ := shFileOperationW.Call(uintptr(unsafe.Pointer(op)))
runtime.KeepAlive(buf)

runtime.KeepAlive compiles to no instructions. It exists purely to be a use of buf that sits after the call, which is enough to stop the collector reclaiming it any earlier.

Why it only showed up in the full test run

This is the part I want to remember.

The window between "buf becomes collectable" and "SHFileOperationW returns" is a few microseconds. To crash, a GC cycle has to start inside it and actually sweep that object. In this package's own tests, almost nothing else is allocating, GC barely runs, and the window closes before anything looks at it.

go test ./... runs packages in parallel. Thirty-nine packages allocating at once is far more GC pressure than one, and eventually a cycle lands where it hurts.

So a single crash wasn't telling me the bug was rare. It was telling me how often the garbage collector happened to run in that window. Those are completely different facts, and the first one is very easy to file under "flaky test" and move on.

The tradeoff

I stopped using the dependency and wrote the Recycle Bin call myself, about sixty lines including the struct layout and its comments. That's more code I own, and the Win32 struct field order is now my problem if it ever changes.

I did it because the bug is unfixable from outside the package, and because a vendored copy with a patch is a thing you forget about. The replacement carries a long comment explaining exactly why KeepAlive is there, so nobody deletes it in a tidy-up. A line that compiles to nothing and looks like it does nothing gets removed eventually, unless the file tells you what it costs.

The takeaway

If you write uintptr(unsafe.Pointer(x)) and store the result anywhere other than a direct argument of the call itself, x needs a runtime.KeepAlive after the call. Struct fields, slices of arguments, a variable you use two lines later: all the same problem.

And if something only crashes under load, check whether "load" means the garbage collector runs more often. That was the whole story here.

The Win32 wrapper with its comment intact is in internal/clean/recyclebin_windows.go. There's more on how the rest of the delete engine works, tombstones and all, in the Devpit case study — and if you need this kind of care applied to something with a browser in front of it, that's what I do for a living.