Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Data Race Patterns in Go

eng.uber.com

Post reply on HN
Print view View original

41–50 of 205 posts

Re: Data Race Patterns in Go

#41

Post by Philip-J-Fry »

masklinn wrote: post #32
Philip-J-Fry wrote: post #23

Earlier quoted context omitted.

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…

I’m a bit doubtful as what you talk about is definitely a slice issue but it’s already an issue in completely sequential code if you reuse appended-to slices. So while it’s also an issue in concurrent code, it’s really no more so.

It is an issue in sequential code because as you say, that's just how slices work. But if you're always using the same variable you'll never encounter it because that slice can't change between you reading that variable and writing to it.

Once concurrency is introduced you can now read from the same variable, but another goroutine may have written to the same slice in the meantime. That's why you must protect the read and writes and synchronise them.

It's fundamentally just a race condition issue with unprotected reads. But people often overlook it in the case of slices because they think they're just taking a reference to the slice, which is safe to do concurrently IF slices were reference types. But they're not, they are copied.

Re: Data Race Patterns in Go

#42

Post by hintymad »

> and contains approximately 2,100 unique Go services (and growing).

A side topic: this is really not something to be proud of. There used to be more people than quantity of work in Uber and engineers fought for credits by building bogus decomposed services, and the sheer number of services seems indicate it's still so.

Re: Data Race Patterns in Go

#43

Post by kjksf »

travisd wrote: post #4

Worth noting that some of these can be detected statically -- and some are detected by go vet (e.g., passing a sync.Mutex by value). I don't think it detects the wg.Add bug, but that seems relatively straightforward(†) to add a check for. (†famous last words, I know)

staticcheck has a check for wg.Add misuse (https://staticcheck.io/docs/checks, https://staticcheck.io/docs/checks#SA2000)

Re: Data Race Patterns in Go

#44

Post by Groxx »

kosherhurricane wrote: post #37
metadat wrote: 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 reference to the slice was resized in the middle of an append operation from another async routine. > What exactly happens in these cases? Go's append looks like this: mySlice = append(mySlice, newItem) To me, this makes it very clear that 1) mySlice pointer can now point to someplace entirely different in memory, and 2) there maybe new allocation. I write both Java and Go. For personal projects, I always cho…

The append pattern also implies the opposite of reality, in that it also (usually!) mutates mySlice. Which is the source of one of the two(?) possible races in that piece of code.

Re: Data Race Patterns in Go

#45

Post by [deleted] »

hintymad wrote: post #42

> and contains approximately 2,100 unique Go services (and growing). A side topic: this is really not something to be proud of. There used to be more people than quantity of work in Uber and engineers fought for credits by building bogus decomposed services, and the sheer number of services seems indicate it's still so.

[deleted]

Re: Data Race Patterns in Go

#46

Post by [deleted] »

metadat wrote: 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…

[deleted]

Re: Data Race Patterns in Go

#47

Post by hintymad »

metadat wrote: 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…

> I never would've guessed that in 2022, Java would start looking more and more appealing in new ways.

I don't quite understand the hatred (to the point of shouting "using Java? Over my dead body), especially in startups, towards Java. I mean, it's a language, big deal. Java's ecosystem more than enough offsets whatever inefficiencies in the language itself, at least for building many of the internal CRUD services. Besides, people like Martin Thompson shows us how to build low-latency applications with ease too. Libraries like JCTools beat the shit out of many new languages when it comes to concurrency for productivity, performance, and reliability. How many engineers in startups claim that they hate Elasticsearch because "Java sucks"? Yet how many can really build a platform as versatile as ES or a Lucene replacement with economical advantages? How many people in startups openly despise Spark or Flink and set out to build a replace because "Java is slow and ugly". Yeah, I've seen a few. And a payment company insists that Rust is the best language because "GC is inefficient and ugly", even though they are still in the phase of product iteration and all their services simply wrap around payment gateways? What's the point?

Disclaimer: I use Go in work. It's not like I have skin in the game for speaking about Java.

Re: Data Race Patterns in Go

#48

Post by smasher164 »

This definitely matches my experience using Go at my previous organization.

1. Closures and concurrency really don't mix well. The loop variable capture in particular is very pernicious. There's an open issue to change this behavior in the language: https://github.com/golang/go/issues/20733.

2. Yep. I've seen this problem in our codebase. I've grown to just be very deliberate with data that needs to be shared. Put it all in a struct that's passed around by its pointer.

3. This issue is caught fairly easily by the race detector. Using a sync.Map or a lock around a map is pretty easy to communicate with other Go devs.

4. This should be documented better, but the convention around structs that should not be passed around by value is to embed a noCopy field inside. https://github.com/golang/go/issues/8005#issuecomment-190753... This will get caught by go vet, since it'll treat it like a Locker.

5 & 6. Go makes it pretty easy to do ad-hoc concurrency as you see fit. This makes it possible for people to just create channels, waitgroups, and goroutines willy-nilly. It's really important to design upfront how you're gonna do an operation concurrently, especially because there aren't many guardrails. I'd suggest that many newcomers stick with x/sync.ErrGroup (which forces you to use its Go method, and can now set a cap on the # of goroutines), and use a *sync.Mutex inside a struct in 99% of cases.

7. Didn't encounter this that often, but sharing a bunch of state between (sub)tests should already be a red flag. Either there's something global that you initialized at the very beginning (like opening a connection), or that state should be scoped and passed down to that individual test, so it can't really infect everything around it.

Re: Data Race Patterns in Go

#49

Post by sidlls »

Groxx wrote: post #44
kosherhurricane wrote: post #37

Earlier quoted context omitted.

>>> The reference to the slice was resized in the middle of an append operation from another async routine. > What exactly happens in these cases? Go's append looks like this: mySlice = append(mySlice, newItem) To me, this makes it very clear that 1) mySlice pointer can now point to someplace entirely different in memory, and 2) there maybe new allocation. I write both Java and Go. For personal projects, I always cho…

The append pattern also implies the opposite of reality, in that it also (usually!) mutates mySlice. Which is the source of one of the two(?) possible races in that piece of code.

I'm no fan of go, although I think it's better than many other languages for services, but the argument here is against the label "append", not the operation. It's a poor name for the operation, but the documentation is quite clear about what's going on. I'd argue that understanding the keywords and builtins of a language is the bare minimum an engineer should do before he starts writing anything in it.

Re: Data Race Patterns in Go

#50

Post by meowface »

Groxx wrote: post #44
kosherhurricane wrote: post #37

Earlier quoted context omitted.

>>> The reference to the slice was resized in the middle of an append operation from another async routine. > What exactly happens in these cases? Go's append looks like this: mySlice = append(mySlice, newItem) To me, this makes it very clear that 1) mySlice pointer can now point to someplace entirely different in memory, and 2) there maybe new allocation. I write both Java and Go. For personal projects, I always cho…

The append pattern also implies the opposite of reality, in that it also (usually!) mutates mySlice. Which is the source of one of the two(?) possible races in that piece of code.

Yes, this majorly tripped me up when working on my first big Go project. Spent days hunting for a non-deterministic data corruption issue which was caused by this. It's definitely my fault for not fully reading the documentation and not realizing that append may (and often does) mutate the slice, but I was indeed misled by the `x = append(x, ...)` syntax into assuming it only works off of a copy without modifying the original.
Post reply on HN