From 997b19545bbf8e5f548eaa87df2f8b87c77e9b4b Mon Sep 17 00:00:00 2001 From: Josh Bleecher Snyder Date: Wed, 16 Mar 2022 17:02:16 -0700 Subject: [PATCH] syncs: use TryLock and TryRLock instead of unsafe The docs say: Note that while correct uses of TryLock do exist, they are rare, and use of TryLock is often a sign of a deeper problem in a particular use of mutexes. Rare code! Or bad code! Who can tell! Signed-off-by: Josh Bleecher Snyder --- cmd/tailscale/depaware.txt | 2 +- cmd/tailscaled/depaware.txt | 2 +- syncs/locked.go | 44 ++++++++----------------------------- 3 files changed, 11 insertions(+), 37 deletions(-) diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 652fe8cbc..f814c8cf1 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -64,7 +64,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep 💣 tailscale.com/net/tshttpproxy from tailscale.com/derp/derphttp+ 💣 tailscale.com/paths from tailscale.com/cmd/tailscale/cli+ tailscale.com/safesocket from tailscale.com/cmd/tailscale/cli+ - 💣 tailscale.com/syncs from tailscale.com/net/interfaces+ + tailscale.com/syncs from tailscale.com/net/interfaces+ tailscale.com/tailcfg from tailscale.com/cmd/tailscale/cli+ W tailscale.com/tsconst from tailscale.com/net/interfaces 💣 tailscale.com/tstime/mono from tailscale.com/tstime/rate diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 9d7b4566a..2d6578031 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -228,7 +228,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/safesocket from tailscale.com/client/tailscale+ tailscale.com/smallzstd from tailscale.com/ipn/ipnserver+ LD 💣 tailscale.com/ssh/tailssh from tailscale.com/wgengine/netstack - 💣 tailscale.com/syncs from tailscale.com/control/controlknobs+ + tailscale.com/syncs from tailscale.com/control/controlknobs+ tailscale.com/tailcfg from tailscale.com/client/tailscale+ W tailscale.com/tsconst from tailscale.com/net/interfaces tailscale.com/tstime from tailscale.com/wgengine/magicsock diff --git a/syncs/locked.go b/syncs/locked.go index f4498a800..e7b5ec311 100644 --- a/syncs/locked.go +++ b/syncs/locked.go @@ -2,58 +2,32 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build go1.13 && !go1.19 -// +build go1.13,!go1.19 - -// This file makes assumptions about the inner workings of sync.Mutex and sync.RWMutex. -// This includes not just their memory layout but their invariants and functionality. -// To prevent accidents, it is limited to a known good subset of Go versions. - package syncs import ( "sync" - "sync/atomic" - "unsafe" -) - -const ( - mutexLocked = 1 - - // sync.Mutex field offsets - stateOffset = 0 - - // sync.RWMutext field offsets - mutexOffset = 0 - readerCountOffset = 16 ) -// add returns a pointer with value p + off. -func add(p unsafe.Pointer, off uintptr) unsafe.Pointer { - return unsafe.Pointer(uintptr(p) + off) -} - // AssertLocked panics if m is not locked. func AssertLocked(m *sync.Mutex) { - p := add(unsafe.Pointer(m), stateOffset) - if atomic.LoadInt32((*int32)(p))&mutexLocked == 0 { + if m.TryLock() { + m.Unlock() panic("mutex is not locked") } } // AssertRLocked panics if rw is not locked for reading or writing. func AssertRLocked(rw *sync.RWMutex) { - p := add(unsafe.Pointer(rw), readerCountOffset) - if atomic.LoadInt32((*int32)(p)) != 0 { - // There are readers present or writers pending, so someone has a read lock. - return + if rw.TryLock() { + rw.Unlock() + panic("mutex is not locked") } - // No readers. - AssertWLocked(rw) } // AssertWLocked panics if rw is not locked for writing. func AssertWLocked(rw *sync.RWMutex) { - m := (*sync.Mutex)(add(unsafe.Pointer(rw), mutexOffset)) - AssertLocked(m) + if rw.TryRLock() { + rw.RUnlock() + panic("mutex is not rlocked") + } }