#Concurrent access to a map within a struct

25 messages · Page 1 of 1 (latest)

civic dust
#

I have a simple struct that contains a map: type S struct { m map[string]bool}.
The struct has a method for reading it: func (s *S) Foo(key string) bool { return s.m[key] }.
It also has a method to update it: func (s *S) Update() { newm := map[string]bool{}; /* computation */ s.m = newm }

My question is: is it safe to use call these functions from independent coroutines without using mutexes to guard the access to m? If I read the Go memory model correctly I think that the answer is yes, because this code is not reading and writing to the same map concurrently: it's accessing a variable that could be updated, and the access to a single variable is atomic, and the compiler will not reorder s.m = newm so that it gets executed before the map is updated. Am I right?

warped fiber
#

If that's the code you have, yes this works

#

go has no surprise, and if you know what the memory model is,
the only information you have to know that maps are fat pointer and not by-value

#

this seems fine as is, is copy-on-write actually better than a mutex there? Up to you

shut root
#

to clarify, you either slap a mutex around that or use an atomic.Pointer

civic dust
#

Than you. Is it unsafe because a map is not guaranteed to be a single pointer? And pointers are not guaranteed to be a single machine word either, I suppose?

shut root
#

they are, but you're relying on the fact that word access is atomic on intel, go provides no such guarantees

#

it is true that you are not sharing the map, but you are sharing s.m

#

wrt to the go memory model, this is a race

#

actually you're probably sharing the map too inadvertently but that's beside the point

warped fiber
#

s.m = newm that's not atomic?

shut root
#

not according to the memory model no

#

you can simplify it to a var global int and reason about that

civic dust
#

For the record, and to redeem myself a bit, I know that the safe thing to do is "just slap a mutex around it", and I would do it anyway, but I was wondering "but do I have to?"

Specifically, the memory model says "Otherwise, each read of a single-word-sized or sub-word-sized memory location must observe a value actually written to that location (perhaps by a concurrent executing goroutine) and not yet overwritten", so if I can assume that a map variable is a word-sized pointer, then it's safe to not wrap reading/writing to that variable, but in practice I may never know if a variable is word-sized or not.

shut root
#

yeah but you jumped the gun a bit

#

there are 2 things to consider here, first, what is a race: "A read-write data race on memory location x consists of a read-like memory operation r on x and a write-like memory operation w on x, at least one of which is non-synchronizing, which are unordered by happens before (that is, neither r happens before w nor w happens before r)."

second: First, any implementation can, upon detecting a data race, report the race and halt execution of the program. Implementations using ThreadSanitizer (accessed with “go build -race”) do exactly this.

civic dust
#

(also perhaps s.m is not a single read, in which case the read access is full into undefined behaviour territory)

shut root
#

that snippet you posted is for implementations that choose not to crash when observing a race

#

so, ignoring how maps work (ie, m[k] is compiled down to mapaccess(m, k) who god knows what it does), your snippet, as it stands today, is at most "probably ok"

warped fiber
#

in practice, it works (at least with gc)

shut root
#

if you run without -race :p

warped fiber
#

I thought the memory model covered more ground and had atomic writes for word sized values

shut root
#

I don't think there are any formal guarantees for gc, but I'm not sure. I suspect it just relies on the architecture. But as the model says, it is free to crash

civic dust
#

Ok, I tried an even simpler snippet of code (where just m is being written by concurrent goroutines) and ThreadSanitizer get very upset at me 🙂