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.