Data race question in config package of harvester

[Sotirios Mantziaris]

i am working on a library for configuration named harvester(https://github.com/taxibeat/harvester), which will be soon properly open sourced, where i can dynamically reconfigure the configuration from an outside system (consul).

The user provides a struct with some tags

type testConfig struct {

Name    string  `consul:"harvester1/name"`

}

the consul key is monitored and when it changes the filed Name is set via reflection `SetString(value)`.

the above runs in a go routine.

The struct/field is propagated via pointer to the rest of the code of the application so that a change in the struct is reflected.

Understandably the race detector fails because the goroutine changes the value and someone else is reading it.

There is always only one that writes and multiple that read.

Worst case you get the old value, best case you get the new value.
Is there a chance that reading and assigning values might not be atomic?

[Jan Mercl]

On Sun, May 26, 2019 at 3:45 PM Sotirios Mantziaris
<smant...@gmail.com> wrote:

> Is there a chance that reading and assigning values might not be atomic?

Accessing a the full multi word value is non-atomic on most, if not
all architectures.

[Sotirios Mantziaris]

Let's assume that the string field Name has the value `Mr. Smith` and we change this to  `Mr. Anderson` in the goroutine, what are the possible values that i could bet on a read?
If these are either `Mr. Smith` or `Mr. Anderson` i am very ok with that because i want the value to be eventually consistent. If there is another possible outcome then i need to synchronize the access and refactor a lot.

[Jan Mercl]

On Sun, May 26, 2019 at 4:03 PM Sotirios Mantziaris
<smant...@gmail.com> wrote:

> Let's assume that the string field Name has the value `Mr. Smith` and we change this to `Mr. Anderson` in the goroutine, what are the possible values that i could bet on a read?
>
> If these are either `Mr. Smith` or `Mr. Anderson` i am very ok with that because i want the value to be eventually consistent.

That would be the only possible outcomes iff a multi word value is
updated atomically.

> If there is another possible outcome then i need to synchronize the access and refactor a lot.

Any outcome is possible with a data race. One of those that are often
seen in practices is, obviously, `Mr. Ander`. Another is that the app
will segfault. Also, the Go memory model does not guarantee one
goroutine will _ever_ observe a change made by a different goroutine
concurrently but without proper synchronization. The compiler if free
to consider all values not explicitly mutated by a code path to never
change without synchronization.

tl;dr: There's no safe way to ignore a data race.

[Wojciech S. Czarnecki]

On Sun, 26 May 2019 07:03:35 -0700 (PDT)
Sotirios Mantziaris <smant...@gmail.com> wrote:

> Let's assume that the string field Name has the value `Mr. Smith` and we
> change this to `Mr. Anderson` in the goroutine, what are the possible
> values that i could bet on a read?
> If these are either `Mr. Smith` or `Mr. Anderson` i am very ok with that
> because i want the value to be eventually consistent. If there is another
> possible outcome then i need to synchronize the access and refactor a lot.

[1] There is no such thing as a benign data race.

Now you use string, down the line someone will 'optimize' code with
[]byte and suddenly you may read `Mr. Smitrson` as well.

Also, for raced parts of the structure, you inevitably will end with
`Mr. Anderson` having Mr. Smith's salary and Mr. Smith having
coat color of Ms. Brown's dog.

Hope that below lecture will help

[1] https://software.intel.com/en-us/blogs/2013/01/06/benign-data-races-what-could-possibly-go-wrong

--
Wojciech S. Czarnecki
<< ^oo^ >> OHIR-RIPE

[Sotirios Mantziaris]

I understand, i have to synchronize access...
Coming from another language i had some guarantees on some assignments mostly int. A string might be a issue here of course...

I have to refactor my code in order to make is safe.

thanks.

[Robert Engels]

Ignore the incorrect comments from the others. There are many valid cases where relaxed concurrency rules apply and you don’t need synchronization just atomic ops (and with certain platform this is not needed - eg java volatile)That is why GC systems can outperform non GC systems in concurrent scenarios. But you need to allocate new objects not modify existing ones (a big reason GC platform strings are usually immutable). You can do the same in Go. 

--
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...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/9abf1f2b-00bf-4346-b429-e40617997237%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Sotirios Mantziaris]

From what i understand you propose to create a new object and switch out the old one with the new one using the atomic package of go.
If that is the case i cannot use it since i don't control the field directly.

I am using the SetString method of the Value struct in the reflect package.

// SetString sets v's underlying value to x.

// It panics if v's Kind is not String or if CanSet() is false.

func (v Value) SetString(x string) {

v.mustBeAssignable()

v.mustBe(String)

*(*string)(v.ptr) = x

}

To unsubscribe from this group and stop receiving emails from it, send an email to golan...@googlegroups.com.

[Jan Mercl]

On Sun, May 26, 2019 at 8:05 PM Sotirios Mantziaris
<smant...@gmail.com> wrote:

> From what i understand you propose to create a new object and switch out the old one with the new one using the atomic package of go.

That cannot work. String is a multi word value. There's nothing in the
atomic package that can update a multi word value. However, a pointer
to anything _can_ be updated atomically.

You cannot "safely" cheat on the data race. As said before, you must
synchronize (the readers vs writers). There's no other option.

[Sotirios Mantziaris]

I was thrown of by the previous comment.
I think i will create some "atomic" types that handle using mutexes with setter and getter methods.

Thanks Jan

[Robert Engels]

I was referring to updating a reference to a string, which can be updated atomically.

This is an advantage that Java offers over Go for concurrent programming. Since everything is reference you don’t face this distinction. Which is why most Go uses channels, which are implemented with locks.

> --
> 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...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAA40n-XTGeeN%3D%3D7R7QMc22KMs_iTWgO4Z%3DWfqoKaFkftenY-7Q%40mail.gmail.com.

[Robert Engels]

Using atomics, which is what I stated to do, avoids the data race and is Edie sully useful with weak atomics. 

--

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...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/57967151-532b-4e32-bbed-f4c0eb2a7038%40googlegroups.com.

[Sotirios Mantziaris]

Thanks for the clarification Robert.

On Sunday, May 26, 2019 at 9:50:56 PM UTC+3, Robert Engels wrote:

I was referring to updating a reference to a string, which can be updated atomically.

This is an advantage that Java offers over Go for concurrent programming. Since everything is reference you don’t face this distinction. Which is why most Go uses channels, which are implemented with locks.

> On May 26, 2019, at 1:16 PM, Jan Mercl <0xj...@gmail.com> wrote:
>
> On Sun, May 26, 2019 at 8:05 PM Sotirios Mantziaris
> <smant...@gmail.com> wrote:
>
>> From what i understand you propose to create a new object and switch out the old one with the new one using the atomic package of go.
>
> That cannot work. String is a multi word value. There's nothing in the
> atomic package that can update a multi word value. However, a pointer
> to anything _can_ be updated atomically.
>
> You cannot "safely" cheat on the data race. As said before, you must
> synchronize (the readers vs writers). There's no other option.
>
> --
> 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 golan...@googlegroups.com.

[Robert Engels]

* especially

[Wojciech S. Czarnecki]

On Sun, 26 May 2019 13:50:13 -0500
Robert Engels <ren...@ix.netcom.com> wrote:

> This is an advantage that Java offers over Go for concurrent programming.
> Since everything is reference you don’t face this distinction. Which is why
> most Go uses channels, which are implemented with locks.

It would be good and fruitful to self educate a bit about Go language, in this
case builtin type string [1], before throwing at a Go newcomer Java's concepts
and ways as ones suitable for Go programming.

FYI. In Go, type string **is a reference type**, it consists of a pointer to the
(immutable) bytes array and a len field.

P.S. To the vigil CoC enforcers: I consider your reprimand as already received:)
Please note that I have used no forbidden t- and s-words, even if they would
be more than appropriate.

Sigh,

[1] https://go101.org/article/string.html

[Robert Engels]

I said nothing to the contrary. I was referring to the structures the poster discussed - which are not heap allocated within an array which differs from Java objects.

I think you would be better served by doing more reading and listening and less yelling.

> --
> 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...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/20190526222528.152fa50e%40zuzia.

[Robert Engels]

And for the OP, there are atomics defined for strings - so what I referred you to is still valid.

> To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/7A09BAC8-4B13-47F3-80BE-39121C4BCFC1%40ix.netcom.com.

[roger peppe]

On Sun, 26 May 2019 at 19:17, Jan Mercl <0xj...@gmail.com> wrote:

On Sun, May 26, 2019 at 8:05 PM Sotirios Mantziaris
<smant...@gmail.com> wrote:

> From what i understand you propose to create a new object and switch out the old one with the new one using the atomic package of go.

That cannot work. String is a multi word value. There's nothing in the
atomic package that can update a multi word value.

https://golang.org/pkg/sync/atomic/#Value ?

That aside, with respect to the original problem, ISTM that changing values under the noses of all those other goroutines wouldn't be that desirable even if it didn't trigger the race detector. Why not provide those goroutines with a way of registering (or obtaining) a channel on which an immutable copy of the current settings can be received? Then they can decide how to incorporate those settings into their current world view.

 

However, a pointer
to anything _can_ be updated atomically.

You cannot "safely" cheat on the data race. As said before, you must
synchronize (the readers vs writers). There's no other option.

--
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...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAA40n-XTGeeN%3D%3D7R7QMc22KMs_iTWgO4Z%3DWfqoKaFkftenY-7Q%40mail.gmail.com.

[Dan Kortschak]

Please don't. Java is not relevant here and the advice given by other
prior to this post in the thread is not incorrect. Using atomic
operations (in this case it would be atomic.Value's Load and Store
methods) invokes a write barrier, which is fundamentally just a memory
synchronisation. In pkg sync there is good advice in the Overview of
the package, though the best advice is in the MM https://golang.org/ref
/mem#tmp_1:

> Programs that modify data being simultaneously accessed by multiple
> goroutines must serialize such access.
>
> To serialize access, protect the data with channel operations or
> other synchronization primitives such as those in the sync and
> sync/atomic packages.
>
> If you must read the rest of this document to understand the behavior
> of your program, you are being too clever.
>
> Don't be clever.

On Sun, 2019-05-26 at 12:51 -0500, Robert Engels wrote:
> Ignore the incorrect comments from the others. There are many valid
> cases where relaxed concurrency rules apply and you don’t need
> synchronization just atomic ops (and with certain platform this is
> not needed - eg java volatile)That is why GC systems can outperform
> non GC systems in concurrent scenarios. But you need to allocate new
> objects not modify existing ones (a big reason GC platform strings
> are usually immutable). You can do the same in Go. 
>
> >

> > On May 26, 2019, at 11:17 AM, Sotirios Mantziaris <smantziaris@gmai

[robert engels]

They are not clever. They are foundational in high performance lock-free structures and other techniques commonly found in HPC and HFT - and even the linux kernel (RCU). This is a decent overview https://www.cs.cmu.edu/~410-s05/lectures/L31_LockFree.pdf but a bit hard to follow since it is slides.

You can review github.com/robaho/go-concurrency-test to see the performance difference lock-free techniques can offer in certain applications.

For certain, most programs do not need these techniques but dissuading someone from understanding and/or using them because they are “being clever” is not appropriate.

[Dan Kortschak]

Please don't say I've said things I didn't. I did not dissuade someone
from understanding or using something. I quoted the memory model, and I
pointed out that the advice given by previous posters was not incorrect
as you claim. What you are reading them as saying may be wrong, but not
what they are actually saying.

[robert engels]

I’m sorry if you think I put words in your mouth, I did not mean to.

Can you please explain what "Please don’t” means then? I took it at face value, and that it was a affirmative response to “Don’t be clever."

> To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/84ac843e670e7781198d231193cd45908d8b58b1.camel%40kortschak.io.

[Dan Kortschak]

In the post that I was replying to you told the OP to ignore incorrect
comments and then brought in the java world and stated that
synchronisation was not needed, only atomic operations. The comments by
previous posters were not incorrect and how things work in java is only
barely relevant here given the way that that language handles objects
and how that that differs from how Go handles values. The claim that
synchronisation is not required is arguably correct; the use of atomics
in Go imposes a write barrier, which is a form of synchronisation.

On the being clever issue, the benefits that are gained from using
lower level synchronisation constructs and building from there rather
than using the language-idiomatic constructs depend on context. In
cases they are definitely warranted, in others not so much. Given that
the OP was asking about issues that indicated that there was some
misunderstanding about how race conditions affect Go, using lower level
constructs seems like something that should not be happening now (maybe
later if warranted by performance needs). Given that the aspect of the
project that is being asked about is in re-configuration, it is likely
not to be on a hot path and so probably (I have not looked at the code
in depth) will never need to have that level of performance
optimisation. From the OP, the sensible approach looks like a
sync.RWLock-protected struct with getters and setters.

[Robert Engels]

You have conveniently omitted much of the argument, for example, like reading Ander from a string containing Anderson due to improper synchronization - which is clearly incorrect and what I was pointing out. The java refs are important to this discussion since when everything is ref and under GC, the ABA problem does not occur. Since Go has value structures and arrays of them the issue is dependent upon the detailed data structures as to what concurrency techniques are suitable.

The op specifically asked about valid values that could be seen. It had nothing to do with being clever or over optimizing. As in the slides I included, just saying use a lock or mutex is not correct, as locks have other issues (deadlocks, priority inversion, others ).

Lastly, if you are going to argue against the points, feel free to do so, but incorrectly changing the argument or my position is not very welcome.

[Dan Kortschak]

Jan's comments about possible outcomes other than those outlined in
Sotirios' second post are correct. Using atomic.Value would synchronise
the values due to the introduction of a write barrier. I'm not omitting
much of the argument, you're just missing it.

Sorry, I'm done.

> > > > > > On May 26, 2019, at 6:59 PM, Dan Kortschak <dan@kortschak.i