Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sync: unlocking an unlocked Mutex panics (should throw) #23039

Closed
rhysh opened this issue Dec 7, 2017 · 3 comments
Closed

sync: unlocking an unlocked Mutex panics (should throw) #23039

rhysh opened this issue Dec 7, 2017 · 3 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Go1.11

Comments

@rhysh
Copy link
Contributor

rhysh commented Dec 7, 2017

What version of Go are you using (go version)?

go version go1.9.2 darwin/amd64
go version devel +28736053ad Wed Dec 6 15:50:04 2017 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

darwin/amd64, and the Go playground

What did you do?

defer func() { recover() }()
new(sync.Mutex).Unlock()

sync.Mutex: https://play.golang.org/p/7MFt4Gn037
sync.RWMutex: https://play.golang.org/p/YWnu02fr4X

What did you expect to see?

I expected unlocking an unlocked sync.Mutex to lead to an unrecoverable fatal error, as it did in Go 1.8 and as unlocking an unlocked sync.RWMutex does today.

What did you see instead?

A couple of calls to throw were converted to panic calls as part of 0556e26, apparently unintentionally undoing a portion of 40d81cf.

$ cat /tmp/bug.go 
package main

import "sync"

func main() {
        defer func() { recover() }()
        new(sync.Mutex).Unlock()
}
$ go1.7 run /tmp/bug.go
$ go1.8 run /tmp/bug.go
fatal error: sync: unlock of unlocked mutex

goroutine 1 [running]:
runtime.throw(0x10696e9, 0x1e)
        /usr/local/go/src/runtime/panic.go:596 +0x95 fp=0xc42003ff20 sp=0xc42003ff00
sync.throw(0x10696e9, 0x1e)
        /usr/local/go/src/runtime/panic.go:585 +0x35 fp=0xc42003ff40 sp=0xc42003ff20
sync.(*Mutex).Unlock(0xc42000e0d0)
        /usr/local/go/src/sync/mutex.go:113 +0xa4 fp=0xc42003ff68 sp=0xc42003ff40
main.main()
        /tmp/bug.go:7 +0x57 fp=0xc42003ff88 sp=0xc42003ff68
runtime.main()
        /usr/local/go/src/runtime/proc.go:185 +0x20a fp=0xc42003ffe0 sp=0xc42003ff88
runtime.goexit()
        /usr/local/go/src/runtime/asm_amd64.s:2197 +0x1 fp=0xc42003ffe8 sp=0xc42003ffe0
exit status 2
$ go1.9 run /tmp/bug.go
$ go-tip run /tmp/bug.go
@bradfitz
Copy link
Contributor

bradfitz commented Dec 7, 2017

Probably too late for Go 1.10, especially since we shipped Go 1.9 with this, but I'm not overly concerned if it goes into Go 1.10 either.

/cc @dvyukov @ianlancetaylor

@bradfitz bradfitz added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Dec 7, 2017
@bradfitz bradfitz added this to the Go1.11 milestone Dec 7, 2017
@bradfitz
Copy link
Contributor

bradfitz commented Dec 7, 2017

Also @rsc.

@gopherbot
Copy link

Change https://golang.org/cl/82656 mentions this issue: sync: throw, not panic, for unlock of unlocked mutex

@golang golang locked and limited conversation to collaborators Dec 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
3 participants
@bradfitz @rhysh @gopherbot