-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
proposal: sync/v2: prohibit unlocking mutex in a different goroutine #9201
Comments
It's not a programming error today. The docs say: http://golang.org/pkg/sync/#Mutex.Unlock "A locked Mutex is not associated with a particular goroutine. It is allowed for one goroutine to lock a Mutex and then arrange for another goroutine to unlock it." Also it can be in third-party code. So even if you detect it, you may not be able to change it. |
Re #6: what you say makes sense, but I think that benefits do not outweigh drawbacks in this case. It does not look exactly the same as restricting all GUI calls to the main thread. And we do not take this possibility away, more complex patterns are still possible with channels. Mutex is for: mu.Lock() defer mu.Unlock // something |
It should be easy enough for the compiler and/or static analysis tools to see, for most uses of mutexes, that the mutex is locked and unlocked in the same goroutine. So I believe that HLE is entirely implementable in the common case, which is the only case in which it can be plausibly be used anyhow. It should also be straightforward for the compiler to annotate lock/unlock in the same goroutine for the benefit of dynamic analysis, if that seems useful. Not that I'm necessarily opposed to this idea, but I think you need stronger arguments. Labels changed: added repo-main, release-none. |
Is it really easy to do? I see at least 3 problems: 1. If you do obj.subobj.mu.Lock/obj.subobj.mu.Unlock, compiler must prove that it's the same mutex in both cases, that is, that obj.subobj pointer has not changed. Since these objects are shared (they contain a mutex), it may be not that simple. 2. If compiler sees: mu.Lock() ... mu.Unlock() and proves mu refers to the same object, it can be the case that the situation is actually: mu.Lock() ... // pass responsibility to unlock to a different goroutine ... // receive responsibility to unlock from yet another goroutine ... mu.Unlock() 3. Non-trivial control flow like: func foo() *T { t := ... ... if ... { t.mu.Lock() } ... return t } t := foo() ... if ... { t.mu.Unlock() } ... |
There is nothing to resolve. With the new rules, there must be pairing lock/unlock. Even if identity of obj.subobj.mu has changed in the function, there still must be a pairing unlock for the original value of obj.subobj.mu and a pairing lock for the new value of obj.subobj.mu somewhere else. For HLE and dynamic analysis you don't need to find pairs statically, they will just manifest themselves at runtime. |
I guess I don't grasp how that would work. You lock a mutex presumably with an XACQUIRE instruction. You make a system call. You block. The scheduler runs. You unblock. You come back in a different thread. You unlock the mutex with an XRELEASE instruction. The unlock fails. Execution resumes back at the XACQUIRE lock. But you're running on a different core. Can that really work? My point is that you need to know what happens between the XACQUIRE and the XRELEASE. Or so it seems to me. |
XACQUIRE/XRELEASE transparently fallback on real locking if transactions fail using some heuristics. If you use RTM (restricted transaction memory), then you fallback to locking manually using explicit heuristics if you see something that you can't handle (e.g. a syscall). But this happens at runtime, no prior static knowledge required. Intel prototyped libpthread support for HLE where you could switch any C program that uses pthread_mutex to HLE. And it worked. |
OK, if that is how it works, then why can't we use the same heuristics to fallback if the mutex unlock happens in a different goroutine? By definition we must be on the same core/thread, or we would have already done a fallback when we entered the scheduler. (To be clear, I'm not saying you're wrong, I'm just saying that I don't understand.) |
Yes, we can fallback on normal locking with current mutex semantics. It will work. But good heuristics are critical for efficient HLE. With the proposed new semantics we permanently fall back to locking iff encounter something that we can't handle within transaction (e.g. a syscall). But unlock in a different thread will look like normal contention (it will be detected earlier when the goroutine passes responsibility to unlock mutex to a different goroutine), so condition for permanent fallback to locking becomes moot; and if we don't fallback (always try to execute transactionally several times first), we will waste lots of work. Static analysis don't have dynamic information, so it won't work for static analysis. Dynamic analysis may or may not work, I don't know yet how to implement it. |
Yes, we can fallback on normal locking with current mutex semantics. It will work. But good heuristics are critical for efficient HLE. With the proposed new semantics we permanently fall back to locking iff encounter something that we can't handle within transaction (e.g. a syscall). But unlock in a different thread will look like normal contention (it will be detected earlier when the goroutine passes responsibility to unlock mutex to a different goroutine), so condition for permanent fallback to locking becomes moot; and if we don't fallback (always try to execute transactionally several times first), we will waste lots of work. Static analysis don't have dynamic information, so it won't work for static analysis. Dynamic analysis may or may not work, I don't know yet how to implement it. |
@dvyukov, we had a case for passing locked objects around in C++, we didn't want the object to be unlocked while passed around. So there are cases where we might want to lock in one goroutine and unlock in another one I think. Feel free to email me for details. |
@ghasemloo I believe there are cases like this, but it does not mean you have to use Mutex for them. chan will do just fine. |
Here's a solution to a deadlock case I had recently that requires unlocking in a different goroutine:
One of the senders called in an independent HTTP POST handling function:
Normally a send on c.move causes some reloads and checks of a changed database row, but in this case the race is where a move happens while a timeout detection (triggered by a timer instead of an HTTP request) is causing a teardown of the goroutine. I don't know the quality of my design, but here's the one case I have where lock/unlock has to happen in separate goroutines. |
@pciet no, it doesn't require unlocking a Mutex in a different goroutine. See the previous comment: |
@griesemer suggests that perhaps instead of changing the existing |
Unless switching to Go2 includes rewriting all of Go code from scratch (as far as I understand it doesn't), this won't give any significant benefit. If Lock/Unlock stay and provide the current semantics, we may not do this at all. The point is that 99% Mutex uses already comply. |
Another potential optimization that becomes possible is priority boosting inside of critical sections. Namely, if a goroutine is preempted inside of a critical sections, scheduler may give it another tiny time slot to finish the critical section (or perhaps it can then voluntarily switch in Unlock). Solaris did this for a long time. |
If people convert over time to a On the other hand, if we change the meaning of |
You could create your own |
sync.Mutex allows lock/unlock in different goroutines:
http://golang.org/pkg/sync/#Mutex.Unlock
"A locked Mutex is not associated with a particular goroutine. It is allowed for
one goroutine to lock a Mutex and then arrange for another goroutine to unlock it."
And the same for RWMutex:
http://golang.org/pkg/sync/#RWMutex.Unlock
The possibility to unlock the mutex in a different goroutine is very rarely used in real
code. And if you really need something more complex than lock/unlock in a single
goroutine, you can always use channels.
But it creates several problems:
sections".
We should prohibit possibility to unlock in a different goroutine in Go2.
The text was updated successfully, but these errors were encountered: