5 ms·

The code presented is not thread safe. It reads and writes to Counter.c from two different goroutines. It's easy to make this mistake in Go as it's trivial to

by eis 14y ago

The code presented is not thread safe. It reads and writes to Counter.c from two different goroutines. It's easy to make this mistake in Go as it's trivial to make your program not only concurrent but also parallel if it utilizes more than one thread (needs runtime.GOMAXPROCS(), which will go away in the future so will be the default). If you use goroutines, always make sure that you are not accessing data from more than one goroutine at a time unless you are using lock or atomic instructions (which would be easy in case of a counter). The alternative is to think of passing ownership of data when transferring it over a channel. The receiver is now in charge of it and should be the only one accessing it.

bad_user 14y ago
I'm having a hard time understanding the benefits of goroutines compared to Actors on top of the JVM. I never played with Go, but I was somehow under the impression that goroutines behave like processes and thus the only way to share data being through messages.
- eis 14y ago
  Have you used coroutines and threads before? Goroutines are basically coroutines multiplexed on threads. If you want to, you can even just think of them as super lightweight threads. They are not like processes, they share the same address space.
  - luriel 14y ago
    > They are not like processes, they share the same address space. Yes, but is worth remembering Go's (concurrency) motto: Do not communicate by sharing memory; instead, share memory by communicating. With channels you can send values, or pointers to the shared address space, so is much more efficient, or even channels (channels of channels are a very powerful and useful concept).
    - eis 14y ago
      Most of the time, following this principle will make sense. It should be also said though that it's not always the right choice. There are situations where using locks or atomic instructions is the better choice. But they nearly only come up in performance critical contexts. It would be very nice to have some kind of compile-time enforceable ownership of data regarding channels and goroutines because debugging thread-safety issues is a pita. I don't know how feasable it is to implement this though.
      - pcwalton 14y ago
        It is feasible if the language is carefully designed around it. Rust implements this through its uniqueness typing system. It is impossible to have two threads share mutable data.
- luriel 14y ago
  In one word: channels. They might be theoretically equivalent to some degree, but in practice they are quite different, and I find reasoning about both concurrency and parallelism using goroutines and channels much more natural. Also see this talk by Rob Pike on the origins of Go's concurrency model here: http://go-lang.cat-v.org/talks/ http://go-lang.cat-v.org/talks/ See also this article by Russ Cox about CSP: http://swtch.com/~rsc/thread/ http://swtch.com/~rsc/thread/ And Rob Pike's latest talk about concurrency patterns at Google IO last week: http://www.youtube.com/watch?v=f6kdp27TYZs http://www.youtube.com/watch?v=f6kdp27TYZs
jgrahamc 14y ago
Darn. I just knew that when I was extracting some showable bits of code from Railgun and making simple examples that I'd mess something up. The third Gist in there isn't anything like the Railgun code (it was just showing the use of the Counter object) and, as you point out, it is not thread safe. I will update the blog post with a version that uses a channel and select to send the count.
- ralph 14y ago
  Have you done the update now? Only there's a NewCounter() that's called but not defined and the text says "it can be passed to counter.New".
- ralph 14y ago
  Hi John, https://gist.github.com/3039932 https://gist.github.com/3039932 has problems; w.Count() is evaluated every time around the for-loop even if the result isn't sent down the count channel. See http://play.golang.org/p/pbCX13my0q http://play.golang.org/p/pbCX13my0q where I've altered lines 60-62 yet there's still only "29 bytes written".
  - jgrahamc 14y ago
    Yes, that's correct. It does look a bit odd doesn't it. As you say it's because the w.Count() gets evaluated by the select each time around the loop and so the count gets reset to 0. Probably the best way to fix that is to make Count() non-destructive and have an explicit Clear function that gets called when a count has been delivered. Like this: http://play.golang.org/p/LWVvdhwBiQ http://play.golang.org/p/LWVvdhwBiQ Since this blog post is likely to be around for a while I will update the code.
    - ralph 14y ago
      It's almost like sometimes it would have handy to do case ch <- defer foo(): and only have foo() be evaluated if ch is ready for writing and also chosen amongst the cases. As the (fixed) code stands either the Sprintf() and Sum() or Count() is wasted on each iteration. I suppose in some cases you can calculate the initial values before the for-loop and then replenish them in the case: when they're used.

by eis 14y ago

bad_user 14y ago

I'm having a hard time understanding the benefits of goroutines compared to Actors on top of the JVM. I never played with Go, but I was somehow under the impression that goroutines behave like processes and thus the only way to share data being through messages.

eis 14y ago
Have you used coroutines and threads before? Goroutines are basically coroutines multiplexed on threads. If you want to, you can even just think of them as super lightweight threads. They are not like processes, they share the same address space.
- luriel 14y ago
  > They are not like processes, they share the same address space. Yes, but is worth remembering Go's (concurrency) motto: Do not communicate by sharing memory; instead, share memory by communicating. With channels you can send values, or pointers to the shared address space, so is much more efficient, or even channels (channels of channels are a very powerful and useful concept).
  - eis 14y ago
    Most of the time, following this principle will make sense. It should be also said though that it's not always the right choice. There are situations where using locks or atomic instructions is the better choice. But they nearly only come up in performance critical contexts. It would be very nice to have some kind of compile-time enforceable ownership of data regarding channels and goroutines because debugging thread-safety issues is a pita. I don't know how feasable it is to implement this though.
    - pcwalton 14y ago
      It is feasible if the language is carefully designed around it. Rust implements this through its uniqueness typing system. It is impossible to have two threads share mutable data.
luriel 14y ago
In one word: channels. They might be theoretically equivalent to some degree, but in practice they are quite different, and I find reasoning about both concurrency and parallelism using goroutines and channels much more natural. Also see this talk by Rob Pike on the origins of Go's concurrency model here: http://go-lang.cat-v.org/talks/ http://go-lang.cat-v.org/talks/ See also this article by Russ Cox about CSP: http://swtch.com/~rsc/thread/ http://swtch.com/~rsc/thread/ And Rob Pike's latest talk about concurrency patterns at Google IO last week: http://www.youtube.com/watch?v=f6kdp27TYZs http://www.youtube.com/watch?v=f6kdp27TYZs

jgrahamc 14y ago

Darn. I just knew that when I was extracting some showable bits of code from Railgun and making simple examples that I'd mess something up. The third Gist in there isn't anything like the Railgun code (it was just showing the use of the Counter object) and, as you point out, it is not thread safe. I will update the blog post with a version that uses a channel and select to send the count.

ralph 14y ago
Have you done the update now? Only there's a NewCounter() that's called but not defined and the text says "it can be passed to counter.New".
ralph 14y ago
Hi John, https://gist.github.com/3039932 https://gist.github.com/3039932 has problems; w.Count() is evaluated every time around the for-loop even if the result isn't sent down the count channel. See http://play.golang.org/p/pbCX13my0q http://play.golang.org/p/pbCX13my0q where I've altered lines 60-62 yet there's still only "29 bytes written".
- jgrahamc 14y ago
  Yes, that's correct. It does look a bit odd doesn't it. As you say it's because the w.Count() gets evaluated by the select each time around the loop and so the count gets reset to 0. Probably the best way to fix that is to make Count() non-destructive and have an explicit Clear function that gets called when a count has been delivered. Like this: http://play.golang.org/p/LWVvdhwBiQ http://play.golang.org/p/LWVvdhwBiQ Since this blog post is likely to be around for a while I will update the code.
  - ralph 14y ago
    It's almost like sometimes it would have handy to do case ch <- defer foo(): and only have foo() be evaluated if ch is ready for writing and also chosen amongst the cases. As the (fixed) code stands either the Sprintf() and Sum() or Count() is wasted on each iteration. I suppose in some cases you can calculate the initial values before the for-loop and then replenish them in the case: when they're used.