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

51–60 of 205 posts

Re: Data Race Patterns in Go

#51

Post by 0xFACEFEED »

smasher164 wrote: post #48

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 i…

> 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.

I run my Go development server with the -race flag as a default. If it affects performance I'll turn it off but that's very rare in practice. Unfortunately a lot of applications don't run tests against their HTTP endpoints (like only internal library stuff) which is bad bad bad, but the -race flag at least helps mitigate.

To anyone reading who cares:

1) Always run your tests with the -race flag!

2) Always write tests for your HTTP handling code too!

3) Run your dev server with -race for a week and see what happens.

This will hard crash your Go program and there is nothing you can do about it. You can't recover(). Go vet will not catch anything. The -race flag will!

  package main

  import "time"

  func main() {
   m := map[int]int{}
   go poop(m)
   go poop(m)
   time.Sleep(5 * time.Second)
  }

  func poop(m map[int]int) {
   for i := 0; i

Re: Data Race Patterns in Go

#52

Post by avgcorrection »

bumper_crop wrote: post #3

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…

This is Atomic*

* Just don’t be an idiot. Worse is better.

Re: Data Race Patterns in Go

#53

Post by hexxagone »

Groxx wrote: post #27
bumper_crop wrote: post #3

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 q…

Go does not have threads but something like "tasks". The fact that no thread handle is exposed allows for transparently moving these tasks across threads if the scheduler decides so.

"go makes concurrency a first-class concept" I think it usually refers to goroutines being built in the language.

"Go is abnormally dangerous when it comes to concurrency IMO". Personnally, it has not been my experience with Go concurrency. However I have hit some issues when trying to ocrhestrate tasks via channels and ended up resorting to atomics to do the job.

Re: Data Race Patterns in Go

#55

Post by saghm »

hexxagone wrote: post #53
Groxx wrote: post #27

Earlier quoted context omitted.

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 q…

Go does not have threads but something like "tasks". The fact that no thread handle is exposed allows for transparently moving these tasks across threads if the scheduler decides so. "go makes concurrency a first-class concept" I think it usually refers to goroutines being built in the language. "Go is abnormally dangerous when it comes to concurrency IMO". Personnally, it has not been my experience with Go concurren…

> Go does not have threads but something like "tasks". The fact that no thread handle is exposed allows for transparently moving these tasks across threads if the scheduler decides so.

This doesn't stop there being "task handles" then, though? I think the point GP was making is that something that in most languages would be simple methods on a handle like "wait for this task to finish" or "stop this task" instead need to be done manually in Go with channels (or potentially `Context` in the latter case, although that was a later addition to the standard library). It doesn't really matter whether you call it a thread or a task; either way, it would be nice to get some return value from spawning some background operation and being able to use it to directly interact with it. I agree with GP that it does seem like an odd omission, since I haven't really heard any actual practical explanation for it.

Re: Data Race Patterns in Go

#57

Post by collinvandyck76 »

aaronbee wrote: post #20
masklinn wrote: post #11

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…

Yea I think your read on the intent here is correct. I re-read that part five times wondering what I wasn't getting.

Re: Data Race Patterns in Go

#58

Post by jeremy_wiebe »

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.

That number really store out to me too. I’d be very curious how they decide what becomes a separate service.

I’d also be curious if the 50m lines of code included generated code.

Re: Data Race Patterns in Go

#59

Post by DominoTree »

Seems like Rob Pike and co may have failed

"The key point here is our programmers... They’re not capable of understanding a brilliant language... So, the language that we give them has to be easy for them to understand"

Re: Data Race Patterns in Go

#60

Post by erik_seaberg »

sidlls wrote: post #49
Groxx wrote: post #44

Earlier quoted context omitted.

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.

  s2 := append(s1, x)
  s3 := append(s1, y)
shouldn’t be allowed, because what it’s likely to do is not what anyone meant. In a pass-by-value language, passing a slice or map by value should copy it, append should be a method that returns void, and passing a pointer should be the way to share state and avoid copies.
Post reply on HN