On Sat, Jan 7, 2017 at 7:00 PM, 'Keith Randall' via golang-nuts <
golang-nuts@googlegroups.com> wrote:
>
>
> On Friday, January 6, 2017 at 5:12:54 PM UTC-8, tsuna wrote:
>>
>> Hi there,
>> I have a struct that contains an unexported map protected by a mutex,
>> let’s say something like:
>>
>> type Map struct {
>> mtx sync.Mutex
>> col map[string]interface{}
>> }
>>
>> I want to implement a ForEach method that works in O(1) space and
>> doesn’t hold the mutex while working on each entry. This is what I have:
>>
>> func (m *Map) ForEach(fn func(k, v interface{})) {
>> m.mtx.Lock()
>> for k, v := range m.col {
>> m.mtx.Unlock()
>> fn(k, v)
>> m.mtx.Lock()
>> }
>> m.mtx.Unlock()
>> }
>>
>> (runnable example: https://play.golang.org/p/tX2lCCYWxq)
>>
>> The language spec <https://golang.org/ref/spec#For_statements> says:
>>
>>> 3. The iteration order over maps is not specified and is not guaranteed
>>> to be the same from one iteration to the next. If map entries that have not
>>> yet been reached are removed during iteration, the corresponding iteration
>>> values will not be produced. If map entries are created during iteration,
>>> that entry may be produced during the iteration or may be skipped. The
>>> choice may vary for each entry created and from one iteration to the next.
>>> If the map is nil, the number of iterations is 0.
>>
>>
>> With the caveats mentioned above regarding concurrent
>> deletions/insertions in mind, is this implementation of ForEach correct?
>> Is there a better way?
>>
>>
> I'm not sure I understand what you're trying to guarantee.
>
That the code is race-free. The guarantees on data access are pretty loose
– fn() could be called on a key-value pair while it’s being removed/changed
by another goroutine. That’s okay.
> The code you've shown looks fine, but the devil is in the details of what
> fn does.
>
> The fundamental constraint is that you need to make sure that no two
> concurrent operations, at least one of which is a write, happen to the
> map. Treat iteration as a read. And it isn't one read extending for the
> whole loop. It is one "instantaneous" read each time a new k,v is assigned.
> So fn can read all it wants.
> If fn writes, those writes must be synchronized so that they do not happen
> concurrently with the ForEach goroutine iterating.
> So if fn writes in the same goroutine that ForEach is running in, that is
> fine.
> If fn spawns other goroutines to write to the map, asks other goroutines
> to write to the map via channels, etc. then those accesses must be properly
> synchronized.
> One way to do that is to use the same lock as you used in ForEach around
> those accesses.
>
Yes, that’s exactly what’s happening. The contrived example I showed only
included ForEach(), but I also have Get(k) and Set(k, v) methods on this
struct, which can be called concurrently from other goroutines. Of course
they also acquire the same mutex.
Alright, so thanks for checking this wasn’t insane. The code looks weird,
but it is what it is.
--
Benoit "tsuna" Sigoure
--
You received this message because you are subscribed to the Google Groups
"golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to golang-nuts+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
