Does Non Atomic Load Conflicts With CompareAndSwap?

Getting Help
1

Is reading a variable without atomic conflicts with atomic.CompareAndSwap? I have the following code:

package main

import (
	"fmt"
	"sync"
	"sync/atomic"
)

func main() {
	x := int32(0)
	for i := 0; i < 100000; i++ {
		n := int32(0)
		wg := sync.WaitGroup{}
		wg.Add(2)
		go func() {
			atomic.CompareAndSwapInt32(&n, 1, 2)
			wg.Done()
		}()
		go func() {
			x += n
			wg.Done()
		}()
		wg.Wait()
	}
	fmt.Println(x)
}

It’s very simple program: there are two goroutines, one of them call atomic.CompareAndSwap() on n. The other one reads n (without atomic). Running this program with go run -race main.go gives this output:

==================
WARNING: DATA RACE
Read at 0x00c00001413c by goroutine 8:
  main.main.func2()
      /tmp/atomictest/main.go:20 +0x59

Previous write at 0x00c00001413c by goroutine 7:
  sync/atomic.CompareAndSwapInt32()
      /home/jauhararifin/.gvm/gos/go1.21/src/runtime/race_amd64.s:310 +0xb
  sync/atomic.CompareAndSwapInt32()
      <autogenerated>:1 +0x18

Goroutine 8 (running) created at:
  main.main()
      /tmp/atomictest/main.go:19 +0x4a

Goroutine 7 (running) created at:
  main.main()
      /tmp/atomictest/main.go:15 +0x17c
==================

A data race is detected there because I read n (without atomic) concurrently with atomic.CompareAndSwap(&n, ..., ...). Notice that I set the CAS operation to always fail.

This is understandable because I don’t use atomic operation when reading n. But I read the go sync.Mutex implementation:

func (m *Mutex) Lock() {
	// Fast path: grab unlocked mutex.
	if atomic.CompareAndSwapInt32(&m.state, 0, mutexLocked) {
		if race.Enabled {
			race.Acquire(unsafe.Pointer(m))
		}
		return
	}
	// Slow path (outlined so that the fast path can be inlined)
	m.lockSlow()
}

func (m *Mutex) lockSlow() {
	var waitStartTime int64
	starving := false
	awoke := false
	iter := 0
	old := m.state
	...
}

As you can see, there is a possible concurrent access on the m.state. In the fast path, one goroutine can execute CAS on &m.state. Concurrently, another goroutine can execute old := m.state in the slow path. But having concurrent Lock() with -race flag doesn’t show any report of data race. How is this possible?

2 
if race.Enabled {
        race.Acquire(unsafe.Pointer(m))
    }

This tells the detector that there is no contention problem.

3

But the data race should also happen even when race.Acquire(unsafe.Pointer(m)) is not triggered though. For example, let’s say there are 3 goroutines:

  1. goroutine 1: lock the mutex, successfully in fast path
  2. goroutine 2: lock the mutex in fast path, but failed and went to the slow path
  3. goroutine 2 now is about to execute old := m.state
  4. goroutine 3: lock the mutex, it tries to execute atomic.CompareAndSwap(...) in the fast path
  5. goroutine 1: release the lock
  6. data race here: goroutine 2 now execute old := m.state and goroutine 3 execute atomic.CompareAndSwap(...) concurrently. The race.Acquire(unsafe.Pointer(m)) is not called in case of goroutine 2 and 3.

In above scenario, both goroutine 2 and 3 concurrent access m where one of them is load and the other is CAS, without runtime.Acquire being called. Yet, there is no data race captured.

4

Oh, I misunderstood your question.
I re-read the code you provided and the source code of mutex.

The two examples are essentially different.
You need to understand what atomic.CompareAndSwapInt32 is doing:

  1. Start atomic execution
  2. Read state variable, compare old variable, and write new when there is consistency
  3. End atomic
    The only goroutine that can execute the above is the first goroutine (A) that grabs the lock.

So what are the subsequent goroutines doing?

  1. Start atomic execution
  2. Read state variable, compare old variable, find inconsistency, do not write new
  3. End atomic
  4. Read state variable (old := m.state)
  5. …

Did you find it? The subsequent goroutines only involve read operations, and no write occurs. Concurrent reading of variables will not generate race.

An example of simplification is:

type T struct {
	x int32
}

func (t *T) X() {
	if atomic.CompareAndSwapInt32(&t.x, 0, 1) {
		return
	}
	_ = t.x
	//fmt.Println(t.x)
}
5

Did you find it? The subsequent goroutines only involve read operations, and no write occurs. Concurrent reading of variables will not generate race.

If you check my original code:

n := int32(0)
go func() {
	atomic.CompareAndSwapInt32(&n, 1, 2)
}()
go func() {
	x += n
}()

^ the atomic.CompareAndSwapInt32 above is never succesfully swap the n. Because the n is initiated with 0, and the CAS swap it from 1 to 2. So the CAS is always failed. It only reads n, and do nothing. It never swap the n. In the end of this execution, n will always be guaranteed to be zero.
So, in my original code, there is no write occurs as well. But, Go count it as data race.

And if you see my example above:

  1. goroutine 1: lock the mutex, successfully in fast path
  2. goroutine 2: lock the mutex in fast path, but failed and went to the slow path
  3. goroutine 2 now is about to execute old := m.state
  4. goroutine 3: lock the mutex, it tries to execute atomic.CompareAndSwap(...) in the fast path
  5. goroutine 1: release the lock
  6. data race here: goroutine 2 now execute old := m.state and goroutine 3 execute atomic.CompareAndSwap(...) concurrently. The race.Acquire(unsafe.Pointer(m)) is not called in case of goroutine 2 and 3.

In the step (6) above, both goroutine 2 and 3 can access the same data m.state, where goroutine 3 would write it through CAS, and goroutine 2 will erad it without atomic. And this doesn’t trigger data race detection.

6

emmm, you are right.
This is very interesting. I modified the standard library and added a function:

func(m *Mutex)Test(){
if atomic.CompareAndSwapInt32(&m.state, 0, mutexLocked) {
if race.Enabled {
race.Acquire(unsafe.Pointer(m))
}
return
}
_=m.state
}

It is found that no warning is generated when calling? It seems that the standard library has some ignoring mechanism?

7

This is my first hypothesis. But I have no idea how to proof this.

8

I checked some materials:

and some source code:
src/runtime/race/*
src/runtime/race.go
src/sync/*

It seems that race detection is performed by external gadgets, but I still don’t know how to register specific ignore packages.
It seems that I have to analyze the workflow of goroutine…
But I don’t have the energy to continue exploring at the moment.
But I will wait for other people’s answers.

1 Like
9

I’ve been spending some time reading how generates SSA when -race is enabled. As it turns out, when you read a variable, go will add additional call runtime.raceread instruction in the SSA. This instruction is not added normally. Only when you build with -race flag, it will be added.

The interesting part is that, in mutex.go file (inside the sync package) the runtime.raceread call is not added.

So, I grep go source code, and look for raceread occurence in code generation part. And I found this:

// src/cmd/compile/internal/ssagen/ssa.go
ir.Syms.Raceread = typecheck.LookupRuntimeFunc("raceread")

Then I look at where ir.Syms.Raceread is used, and I found this:

// src/cmd/compile/internal/ssagen/ssa.go
func (s *state) instrument2(t *types.Type, addr, addr2 *ssa.Value, kind instrumentKind) {
...
	} else if base.Flag.Race {
		// for non-composite objects we can write just the start
		// address, as any write must write the first byte.
		switch kind {
		case instrumentRead:
			fn = ir.Syms.Raceread
		case instrumentWrite:
			fn = ir.Syms.Racewrite
		default:
			panic("unreachable")
		}
	} else if base.Flag.ASan {

My intuition says that instrument2 is called when -race is added. So, I look closely on instrument2:

func (s *state) instrument2(t *types.Type, addr, addr2 *ssa.Value, kind instrumentKind) {
	if !s.curfn.InstrumentBody() {
		return
	}

It seems, the instrumentation code is skipped when InstrumentBody() returns false.

func (f *Func) InstrumentBody() bool           { return f.flags&funcInstrumentBody != 0 }
func (f *Func) SetInstrumentBody(b bool)           { f.flags.set(funcInstrumentBody, b) }

So, I look on who calls SetInstrumentBody.

	if !base.Flag.Race || !base.Compiling(base.NoRacePkgs) {
		fn.SetInstrumentBody(true)
	}

The base.NoRacePkgs seems promising, so I look inside:

var NoRacePkgs = []string{"sync", "sync/atomic"}

Yes, it seems sync package is an exception. The instrumentation code is not added there. So even if there is a data race in the sync package, go won’t report them.

2 Likes
10

It seems to be true.
Thank you for answering a question for me.
Before, I was thinking about the detection at runtime, and I didn’t think that the key point was to build in cmd.

Thanks again.