From 288fcb82182282b02102015d988f2d674851e489 Mon Sep 17 00:00:00 2001 From: Ian Gudger Date: Sat, 15 Oct 2022 23:31:11 -0700 Subject: [PATCH] Use standard library sync.Mutex.TryLock. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The standard library sync.Mutex has included a TryLock method since Go 1.18. There is no longer a need to implement a parallel version with unsafe. Using the standard library version reduces the ongoing maintenance cost of this package. https://pkg.go.dev/sync#Mutex.TryLock https://tip.golang.org/doc/go1.18#minor_library_changes --- .github/workflows/go.yml | 2 +- pkg/sync/BUILD | 1 - pkg/sync/mutex_test.go | 72 ---------------------------------------- pkg/sync/mutex_unsafe.go | 35 +++---------------- 4 files changed, 5 insertions(+), 105 deletions(-) delete mode 100644 pkg/sync/mutex_test.go diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 32aa1845f..eff0447b7 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -45,7 +45,7 @@ jobs: fetch-depth: 0 - uses: actions/setup-go@v2 with: - go-version: 1.17 + go-version: 1.18 - run: tools/go_branch.sh - run: git checkout go && git clean -xf . && go build ./... - if: github.event_name == 'push' diff --git a/pkg/sync/BUILD b/pkg/sync/BUILD index d105e9436..78d50f8d9 100644 --- a/pkg/sync/BUILD +++ b/pkg/sync/BUILD @@ -42,7 +42,6 @@ go_test( size = "small", srcs = [ "gate_test.go", - "mutex_test.go", "rwmutex_test.go", "seqcount_test.go", ], diff --git a/pkg/sync/mutex_test.go b/pkg/sync/mutex_test.go deleted file mode 100644 index 4122b2e82..000000000 --- a/pkg/sync/mutex_test.go +++ /dev/null @@ -1,72 +0,0 @@ -// Copyright 2019 The gVisor Authors. -// -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// +checkalignedignore -package sync - -import ( - "sync" - "testing" - "unsafe" -) - -// TestStructSize verifies that syncMutex's size hasn't drifted from the -// standard library's version. -// -// The correctness of this package relies on these remaining in sync. -func TestStructSize(t *testing.T) { - const ( - got = unsafe.Sizeof(syncMutex{}) - want = unsafe.Sizeof(sync.Mutex{}) - ) - if got != want { - t.Errorf("got sizeof(syncMutex) = %d, want = sizeof(sync.Mutex) = %d", got, want) - } -} - -// TestFieldValues verifies that the semantics of syncMutex.state from the -// standard library's implementation. -// -// The correctness of this package relies on these remaining in sync. -func TestFieldValues(t *testing.T) { - var m Mutex - m.Lock() - if got := *m.m.state(); got != mutexLocked { - t.Errorf("got locked sync.Mutex.state = %d, want = %d", got, mutexLocked) - } - m.Unlock() - if got := *m.m.state(); got != mutexUnlocked { - t.Errorf("got unlocked sync.Mutex.state = %d, want = %d", got, mutexUnlocked) - } -} - -func TestDoubleTryLock(t *testing.T) { - var m Mutex - if !m.TryLock() { - t.Fatal("failed to aquire lock") - } - if m.TryLock() { - t.Fatal("unexpectedly succeeded in aquiring locked mutex") - } -} - -func TestTryLockAfterLock(t *testing.T) { - var m Mutex - m.Lock() - if m.TryLock() { - t.Fatal("unexpectedly succeeded in aquiring locked mutex") - } -} - -func TestTryLockUnlock(t *testing.T) { - var m Mutex - if !m.TryLock() { - t.Fatal("failed to aquire lock") - } - m.Unlock() // +checklocksforce - if !m.TryLock() { - t.Fatal("failed to aquire lock after unlock") - } -} diff --git a/pkg/sync/mutex_unsafe.go b/pkg/sync/mutex_unsafe.go index f0a471da4..9bf412700 100644 --- a/pkg/sync/mutex_unsafe.go +++ b/pkg/sync/mutex_unsafe.go @@ -3,62 +3,35 @@ // 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.21 -// +build go1.13,!go1.21 - -// When updating the build constraint (above), check that syncMutex matches the -// standard library sync.Mutex definition. - package sync import ( "sync" - "sync/atomic" "unsafe" ) // CrossGoroutineMutex is equivalent to Mutex, but it need not be unlocked by a // the same goroutine that locked the mutex. type CrossGoroutineMutex struct { - sync.Mutex -} - -type syncMutex struct { - state int32 - sema uint32 -} - -func (m *CrossGoroutineMutex) state() *int32 { - return &(*syncMutex)(unsafe.Pointer(&m.Mutex)).state + m sync.Mutex } // Lock locks the underlying Mutex. // +checklocksignore func (m *CrossGoroutineMutex) Lock() { - m.Mutex.Lock() + m.m.Lock() } // Unlock unlocks the underlying Mutex. // +checklocksignore func (m *CrossGoroutineMutex) Unlock() { - m.Mutex.Unlock() + m.m.Unlock() } -const ( - mutexUnlocked = 0 - mutexLocked = 1 -) - // TryLock tries to acquire the mutex. It returns true if it succeeds and false // otherwise. TryLock does not block. func (m *CrossGoroutineMutex) TryLock() bool { - if atomic.CompareAndSwapInt32(m.state(), mutexUnlocked, mutexLocked) { - if RaceEnabled { - RaceAcquire(unsafe.Pointer(&m.Mutex)) - } - return true - } - return false + return m.m.TryLock() } // Mutex is a mutual exclusion lock. The zero value for a Mutex is an unlocked