Live data from Hacker News

Data Race Patterns in Go

eng.uber.com

21–30 of 205 posts

Re: Data Race Patterns in Go

#21
> We developed a system to detect data races at Uber using a dynamic data race detection technique. This system, over a period of six months, detected about 2,000 data races in our Go code base, of which our developers already fixed ~1,100 data races.

This isn't open source, correct?

Re: Data Race Patterns in Go

#22

Sorry to say, but these hit close to home for me. A lot of the synchronization paradigms in Go are easy to misuse, but lead the author into thinking it's okay. the WaitGroup one is particularly poignant for me, since the race detector doesn't catch it. I'll add one other data race goof: atomic.Value. Look at the implementation. Unlike pretty much every other language I've seen, atomic.Value isn't really atomic, since…

> atomic.Value isn't really atomic, since the concrete type can't ever change after being set. How does this mean it's non-atomic ? As far as I know you can still never Load() a partial Store(). (Also, even if it was possible, this would never be a good idea...)

That's why I opened with "Look at the implementation". Go is unable to store the type and the pointer at the same time, so it warps what "atomic" means. Pretty much every other language has atomic mean "one of these will win, one will lose". Go says "one will win, one will panic and destroy the goroutine.

In fact, it's even worse than that. If the Store() caller goes to sleep between setting the type and storing the pointer, it causes every Goroutine that calls Load() to block. They can't make forward progress if the store caller hangs.

Re: Data Race Patterns in Go

#23
post #8

> 2. Slices are confusing types that create subtle and hard-to-diagnose data races The "Slices" example is just nasty! Like, this is just damning for Go's promise of "_relatively_ easy and carefree concurrency" . Think about it for a second or two, >> The reference to the slice was resized in the middle of an append operation from another async routine. What exactly happens in these cases? How can I trust myself, as…

> The "Slices" example is just nasty! I've got to say I'm not entirely clear on what they talk about specifically. Is it simply that the `results` inside the goroutine will be desync'd from `myResults` (and so the call to myAppend will interact oddly with additional manipulations of results), or is it that the copy can be made mid-update, and `result` itself could be incoherent?

So a slice consists of a pointer to a backing array, a length and a capacity. If you don't use a pointer and pass this slice around you will copy it.

This is problematic because even though you copy it, you're still pointing at the same backing array.

Therefore, a backing array with data like [1,2,3,4,5] could be pointed at by 2 slice headers (slice metadata) looking like

A: {len: 2, cap: 10} [1,2] B: {len:5, cap: 10} [1,2,3,4,5]

So any append operations on slice A will mess up the data in that backing array.

Now, sometimes your append will resize the slice, in which case the data is copied and a slice with a new larger backing array is returned. If this was happening concurrently then you'd lose the data in racing appends.

If the append doesn't need to resize the slice, then you'll overwrite the data in the backing array. And so you'll corrupt the data in the slice.

Here's an example I threw together: https://go.dev/play/p/qRUKUwIf3vx

Although the code in the post doesn't actually look like it has an issue. Their tooling just flagged it up as it potentially has an issue if the copy was actually used in the function. But the `safeAppend` function targets the correct slice each time.

Re: Data Race Patterns in Go

#24
post #9
post #8

> 2. Slices are confusing types that create subtle and hard-to-diagnose data races The "Slices" example is just nasty! Like, this is just damning for Go's promise of "_relatively_ easy and carefree concurrency" . Think about it for a second or two, >> The reference to the slice was resized in the middle of an append operation from another async routine. What exactly happens in these cases? How can I trust myself, as…

Slices may just be one of the best and worst parts of Go. They're cumbersome, their behavior sometimes feels 'inexplicable,' and even as an experienced developer you are likely to eventually fallen into one of the traps where your 'obvious' code isn't so obvious. That said... when programming in programming languages without a slice type, I always want to have one. And though it's confusing at times, the design does…

Indeed. Probably the most common data structure ever used, list of stuff, Go managed to make subtle and full of surprises. A knife without a handle.

This makes the stated reason for the delay of generics hard to understand. They didn't wait to get list/vector/array/slice right.

Re: Data Race Patterns in Go

#25

Earlier quoted context omitted.

> The "Slices" example is just nasty! I've got to say I'm not entirely clear on what they talk about specifically. Is it simply that the `results` inside the goroutine will be desync'd from `myResults` (and so the call to myAppend will interact oddly with additional manipulations of results), or is it that the copy can be made mid-update, and `result` itself could be incoherent?

I believe they made a mistake with that example. It doesn't look unsafe to me because the myResults sliced passed to the goroutine is not used. Or perhaps the racy part was left out of their snippet. Below is what might be what they have meant. This code snippet is racy because an unsafe read of myResults is done to pass it to the goroutine and then that version of myResults is passed to safeAppend: func ProcessAll(u…

[deleted]

Re: Data Race Patterns in Go

#26
post #17

Earlier quoted context omitted.

> What exactly happens in these cases? How can I trust myself, as a fallible human being, to reason about such cases when I'm trying to efficiently roll up a list of results. :-/ For me: minimize shared mutable data. If I really can’t get rid of some shared mutable data, I mutex it or use atomics or similar. This works very well —I almost never run into data races this way, but it is a discipline rather than a techni…

Absolutely, the disappointing part is that as code authors, we need to constantly remember about various (otherwise appealing and even encouraged by the language syntax and control constructs) footguns and "never approach such areas" of (totally valid) syntax. Reminds me of programming in Javascript (it's extreme example, but the similarity is there).

Yeah, it’s a bit disappointing. It doesn’t bother me too much, but it could be improved by a linter which could help you find shared mutable state. Without a concept of “const” (for complex types, anyway), I’m not sure how feasible such a linter would be.

Re: Data Race Patterns in Go

#27

Sorry to say, but these hit close to home for me. A lot of the synchronization paradigms in Go are easy to misuse, but lead the author into thinking it's okay. the WaitGroup one is particularly poignant for me, since the race detector doesn't catch it. I'll add one other data race goof: atomic.Value. Look at the implementation. Unlike pretty much every other language I've seen, atomic.Value isn't really atomic, since…

The lack of generics has forced all Go concurrency to be intrusive (i.e. implemented by the person using literally any concurrency), and yeah. It's horrifyingly error-prone in my experience. It means everyone needs to be an expert, and lol, everyone is not an expert.

Generics might save us from the simple, mechanical flaws. Expect to see `Locker` and `Atomic` types cropping up. And unbounded buffered thread-safe queues backing channels. Etc. I'm very, very much looking forward to it.

--- edited to rant more ---

I also really wonder where all these "go makes concurrency a first-class concept" claims come from, because I see it quite a few places, and I feel like it's making some very strong implied claims that absolutely do not exist.

Go has channels and select. That's neat. But on the other hand it has threads... but no thread handles. It has implicit capturing of closures. It has ambiguous value vs pointer semantics. It (style- and ergonomic-wise) encourages field references, which have no way to enforce mutexes or atomics. It has had crippled lock APIs that effectively force use of channels for... I don't know, philosophical reasons?

Go is abnormally dangerous when it comes to concurrency IMO. The race detector does an amazing job helping you discover it, but it's very easy to not use it or not take full advantage of it (i.e. non-parallel tests), and few run their production services with the race detector enabled. Because if they did, it would crash all the time, because there are an absurd amount of races in nearly all of the popular libraries (and in common use of those libraries, because concurrency is not a first-class citizen and you can't tell when it's happening / when it shouldn't happen).

Re: Data Race Patterns in Go

#28
post #7

This is pretty cool. 50 million lines of code is quite a large corpus to work off of. I'm surprised by some of them. For example, go vet nominally catches misuses of mutexes, so it's surprising that even a few of those slipped through. I wonder if those situations are a bit more complicated than the example. Obviously, the ideal outcome is that static analysis can help eliminate as many issues as possible, by restric…

> I suppose if you want programming language tradeoffs more tilted towards robustness, Rust already has a lot of that covered

Does anyone not want robustness of their language to cover their mistakes?

Re: Data Race Patterns in Go

#29

Earlier quoted context omitted.

> atomic.Value isn't really atomic, since the concrete type can't ever change after being set. How does this mean it's non-atomic ? As far as I know you can still never Load() a partial Store(). (Also, even if it was possible, this would never be a good idea...)

That's why I opened with "Look at the implementation". Go is unable to store the type and the pointer at the same time, so it warps what "atomic" means. Pretty much every other language has atomic mean "one of these will win, one will lose". Go says "one will win, one will panic and destroy the goroutine. In fact, it's even worse than that. If the Store() caller goes to sleep between setting the type and storing the…

This is why all the examples call Store immediately with a zero value of the type.

Re: Data Race Patterns in Go

#30

Earlier quoted context omitted.

> atomic.Value isn't really atomic, since the concrete type can't ever change after being set. How does this mean it's non-atomic ? As far as I know you can still never Load() a partial Store(). (Also, even if it was possible, this would never be a good idea...)

That's why I opened with "Look at the implementation". Go is unable to store the type and the pointer at the same time, so it warps what "atomic" means. Pretty much every other language has atomic mean "one of these will win, one will lose". Go says "one will win, one will panic and destroy the goroutine. In fact, it's even worse than that. If the Store() caller goes to sleep between setting the type and storing the…

[deleted]
Post reply on HN