Skip to content

runtime: fatal error: unlock of unlocked lock (Cleanup.Stop) #81435

Open

Description

@pierrre

Go version

go version go1.27.0-X:nodwarf5 linux/amd64

Output of go env in your module/workspace:

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN='/home/pierre/go/bin'
GOCACHE='/home/pierre/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/pierre/.config/go/env'
GOEXE=''
GOEXPERIMENT='greenteagc'
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build107800302=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/home/pierre/Git/pierrre/go-stuff/go.mod'
GOMODCACHE='/home/pierre/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPACKAGESDRIVER=''
GOPATH='/home/pierre/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='on'
GOTELEMETRYDIR='/home/pierre/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.27.0-X:nodwarf5'
GOWORK='/home/pierre/Git/pierrre/go.work'
PKG_CONFIG='pkg-config'

What did you do?

I can reproduce a runtime crash reliably with this code:

package main

import (
	"fmt"
	"runtime"
	"sync"
)

func main() {
	workers := runtime.GOMAXPROCS(0)
	var wg sync.WaitGroup
	for range workers {
		wg.Go(func() {
			for range 1000000 {
				obj := &[64]byte{}
				var c runtime.Cleanup
				c = runtime.AddCleanup(obj, func(int) {
					c.Stop()
				}, 0)
			}
		})
	}
	wg.Wait()
	fmt.Println("done (no crash)")
}

What did you see happen?

I see this crash

fatal error: unlock of unlocked lock

goroutine 50 gp=0x215922e045a0 m=11 mp=0x215922e00008 [running]:
runtime.throw({0x49f42f?, 0x1?})
        /usr/lib/go/src/runtime/panic.go:1243 +0x48 fp=0x215922f3b6a8 sp=0x215922f3b678 pc=0x479928
runtime.unlock2(0x7f7dd3b21fc0?)
        /usr/lib/go/src/runtime/lock_spinbit.go:319 +0x145 fp=0x215922f3b6e8 sp=0x215922f3b6a8 pc=0x41a005
runtime.unlockWithRank(...)
        /usr/lib/go/src/runtime/lockrank_off.go:36
runtime.unlock(...)
        /usr/lib/go/src/runtime/lock_spinbit.go:270
runtime.Cleanup.Stop({0x215922a77950?, 0x1b?})
        /usr/lib/go/src/runtime/mcleanup.go:268 +0x14d fp=0x215922f3b740 sp=0x215922f3b6e8 pc=0x4239cd
main.main.func1.1(0x423e05?)
        /home/pierre/Git/pierrre/go-stuff/weakcrash/main.go:21 +0x1e fp=0x215922f3b760 sp=0x215922f3b740 pc=0x49b0fe
runtime.callCleanup[...](0x7f7dd392a400?, 0x92)
        /usr/lib/go/src/runtime/mcleanup.go:204 +0x19 fp=0x215922f3b778 sp=0x215922f3b760 pc=0x49b6b9
runtime.AddCleanup[...].func1(0x7f7dd392a400?)
        /usr/lib/go/src/runtime/mcleanup.go:183 +0x25 fp=0x215922f3b7a0 sp=0x215922f3b778 pc=0x49b145
runtime.runCleanups()
        /usr/lib/go/src/runtime/mcleanup.go:745 +0xd4 fp=0x215922f3b7e0 sp=0x215922f3b7a0 pc=0x424494
runtime.goexit({})
        /usr/lib/go/src/runtime/asm_amd64.s:1264 +0x1 fp=0x215922f3b7e8 sp=0x215922f3b7e0 pc=0x47f001
created by runtime.(*cleanupQueue).createGs in goroutine 18
        /usr/lib/go/src/runtime/mcleanup.go:672 +0xa5

What did you expect to see?

It shouldn't crash.

According to the documentation

Stop cancels the cleanup call. Stop will have no effect if the cleanup call has already been queued for execution (because ptr became unreachable). To guarantee that Stop removes the cleanup function, the caller must ensure that the pointer that was passed to AddCleanup is reachable across the call to Stop.

it should be a no-op.

Yes I know my code looks strange, and it doesn't make sense to call Stop() in the cleanup function.

Activity

  1. self-assigned this
    on Sep 11, 2026
  2. added this to the Go1.28 milestone on Sep 11, 2026
  3. prattmic commented on Sep 11, 2026

    @prattmic
    Member

    Thanks for the short reproducer, I can reproduce locally.

  4. added
    NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.
    on Sep 11, 2026
  5. prattmic commented on Sep 11, 2026

    @prattmic
    Member

    Out of curiosity, @pierrre did you encounter this in a real application or found this with a fuzzer, etc?

  6. prattmic commented on Sep 11, 2026

    @prattmic
    Member

    I think I understand what's going on here (thanks @mknyszek for helping!).

    By calling Cleanup.Stop from within the cleanup function, the original object is dead and has been freed.

    Stop is intended to support this case. The normal case here is supposed to be that Stop doesn't find a matching cleanup special for the object, so it just does nothing. Or spanOfHeap simply returns no span because the entire span is dead.

    What happens when this crash occurs:

    1. GC marks no objects in the span, thus the object is dead and cleanup is queued.
    2. Cleanup.Stop starts running, and finds the span because the sweeper hasn't swept it yet.
    3. Cleanup.Stop calls ensureSwept to wait for sweeping to complete.
    4. This races with the background sweeper, which wins. The ensureSwept call spins waiting for sweepgen to increment.
    5. The background sweep "finishes" and updates sweepgen, unblocking ensureSwept.
    6. Cleanup.Stop continues execution and takes span.speciallock.
    7. The sweeper frees the span. freeSpan sets the span state to mSpanDead and puts it on the free list.
    8. Another allocation needs a new span, allocates our span from the free list. mspan.init zeroes mspan.speciallock.
    9. Cleanup.Stop attempts to unlock speciallock but since it has been zeroed it appears unlocked and we throw.

    There are a few bugs here.

    1. Cleanup.Stop takes speciallock and inspects the special list while the span is dead. mspan.init zeroes the lock (and mspan.special) if allocated, which can occur at any time. The most minimal fix may be to avoid zeroing the lock, but I'm not convinced that is the best fix, it's pretty fragile to want to look at specials on dead spans. Instead, I think we should simply avoid looking at dead spans.

    2. spanForHeap already checks if the span is in use, but the span may be dead after ensureSwept, so there should be an extra check after ensureSwept. But ensureSwept only waits for sweepgen to update. Sweeping doesn't set the span to dead until after updating sweepgen. So a check after ensureSwept isn't sufficient without refactoring to ensure the state becomes dead before updating sweepgen [1].

    Other users of ensureSwept don't have this problem because they all hold a live pointer to an object in the span, so sweeping can't free the span.

    [1] Note that once the span becomes dead, it could become live again before ensureSwept returns. s.state.set(mSpanInUse) occurs after zeroing locks, so that is fine. After this it can't become dead again because Cleanup.Stop is non-preemptible, so we can't start a new GC cycle.

  7. pierrre commented on Sep 12, 2026

    @pierrre
    Author

    I encountered this issue while building a "weak map": https://github.com/pierrre/go-libs/tree/main/weakutil

    One of my benchmarks triggered this crash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

NeedsInvestigationSomeone must examine and confirm this is a valid issue and not a duplicate of an existing one.compiler/runtimeIssues related to the Go compiler and/or runtime.

Type

No type

Projects

Relationships

None yet

Development

No branches or pull requests

Issue actions