Non-atomic access to m.state in sync.Mutex

[Maxim Khitrov]

Hi,

I assume that the code is correct, but I'd like to understand why it's
safe. The line "old := m.state" in Mutex.Lock()'s for loop accesses a
variable that could be shared and updated by multiple goroutines, but
it doesn't use atomic.LoadInt32. Is that because aligned 32-bit loads
are atomic on all supported platforms anyway, or is there another
reason?

As a more general question, when are you required to use atomic.Load*
functions to guarantee correct behavior? If m.state was a 64-bit int,
would you have to use LoadInt64 to support 32-bit platforms?

Likewise, what if you have a pointer that's being updated in one
goroutine, but being read in others? The update should probably be
done using Store or CompareAndSwap functions, but do you need an
atomic Load as well?

- Max

[Dmitry Vyukov]

On Fri, Jun 7, 2013 at 4:31 PM, Maxim Khitrov <m...@mxcrypt.com> wrote:
> Hi,
>
> I assume that the code is correct, but I'd like to understand why it's
> safe. The line "old := m.state" in Mutex.Lock()'s for loop accesses a
> variable that could be shared and updated by multiple goroutines, but
> it doesn't use atomic.LoadInt32. Is that because aligned 32-bit loads
> are atomic on all supported platforms anyway, or is there another
> reason?

This is a bug.

> As a more general question, when are you required to use atomic.Load*
> functions to guarantee correct behavior?

Whenever there are concurrent writes to the variable.

> If m.state was a 64-bit int,
> would you have to use LoadInt64 to support 32-bit platforms?
>
> Likewise, what if you have a pointer that's being updated in one
> goroutine, but being read in others? The update should probably be
> done using Store or CompareAndSwap functions, but do you need an
> atomic Load as well?

Yes, one needs atomic.Load.

[罗华清]

the "bug" is still there. any purpose?