From 2278a51fa0570b01f58a80d60589fcf8e33be9c8 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Thu, 28 Apr 2022 21:18:49 -0700 Subject: [PATCH] sync/atomic: use consistent first-store-in-progress marker We need to use the same marker everywhere. My CL to rename the marker (CL 241661) and the CL to add more uses of the marker under the old name (CL 241678) weren't coordinated with each other. Fixes #52612 Change-Id: I97023c0769e518491924ef457fe03bf64a2cefa6 Reviewed-on: https://go-review.googlesource.com/c/go/+/403094 Run-TryBot: Keith Randall Reviewed-by: Ian Lance Taylor TryBot-Result: Gopher Robot Reviewed-by: Keith Randall --- src/sync/atomic/value.go | 8 +++--- test/fixedbugs/issue52612.go | 49 ++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 test/fixedbugs/issue52612.go diff --git a/src/sync/atomic/value.go b/src/sync/atomic/value.go index f18b7ee095..88315f2d88 100644 --- a/src/sync/atomic/value.go +++ b/src/sync/atomic/value.go @@ -101,7 +101,7 @@ func (v *Value) Swap(new any) (old any) { // active spin wait to wait for completion; and so that // GC does not see the fake type accidentally. runtime_procPin() - if !CompareAndSwapPointer(&vp.typ, nil, unsafe.Pointer(^uintptr(0))) { + if !CompareAndSwapPointer(&vp.typ, nil, unsafe.Pointer(&firstStoreInProgress)) { runtime_procUnpin() continue } @@ -111,7 +111,7 @@ func (v *Value) Swap(new any) (old any) { runtime_procUnpin() return nil } - if uintptr(typ) == ^uintptr(0) { + if typ == unsafe.Pointer(&firstStoreInProgress) { // First store in progress. Wait. // Since we disable preemption around the first store, // we can wait with active spinning. @@ -153,7 +153,7 @@ func (v *Value) CompareAndSwap(old, new any) (swapped bool) { // active spin wait to wait for completion; and so that // GC does not see the fake type accidentally. runtime_procPin() - if !CompareAndSwapPointer(&vp.typ, nil, unsafe.Pointer(^uintptr(0))) { + if !CompareAndSwapPointer(&vp.typ, nil, unsafe.Pointer(&firstStoreInProgress)) { runtime_procUnpin() continue } @@ -163,7 +163,7 @@ func (v *Value) CompareAndSwap(old, new any) (swapped bool) { runtime_procUnpin() return true } - if uintptr(typ) == ^uintptr(0) { + if typ == unsafe.Pointer(&firstStoreInProgress) { // First store in progress. Wait. // Since we disable preemption around the first store, // we can wait with active spinning. diff --git a/test/fixedbugs/issue52612.go b/test/fixedbugs/issue52612.go new file mode 100644 index 0000000000..d8be04989f --- /dev/null +++ b/test/fixedbugs/issue52612.go @@ -0,0 +1,49 @@ +// run + +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "sync/atomic" + "unsafe" +) + +var one interface{} = 1 + +type eface struct { + typ unsafe.Pointer + data unsafe.Pointer +} + +func f(c chan struct{}) { + var x atomic.Value + + go func() { + x.Swap(one) // writing using the old marker + }() + for i := 0; i < 100000; i++ { + v := x.Load() // reading using the new marker + + p := (*eface)(unsafe.Pointer(&v)).typ + if uintptr(p) == ^uintptr(0) { + // We read the old marker, which the new reader + // doesn't know is a case where it should retry + // instead of returning it. + panic("bad typ field") + } + } + c <- struct{}{} +} + +func main() { + c := make(chan struct{}, 10) + for i := 0; i < 10; i++ { + go f(c) + } + for i := 0; i < 10; i++ { + <-c + } +} -- 2.48.1