The Go concurrency bug I made

https://dev.to/dannypsnl/the-go-concurrency-bug-i-made-3dma
林子篆 twitter logo github logo May 25 Originally published at dannypsnl.github.io on May 25, 2019 ・3 min read

There is a saying:

I never had a slice of bread particularly large and wide that did not fall upon the floor and always on the buttered side

Even I already work with Go for almost 3 years, I still made these stupid bugs. But learning from errors is why we are professional enigneer. So I’m going to list the bug I made, and show how to avoid it.

1 select with generator

  1. func events() <-chan int {
  2. ch := make(chan int)
  3. go func() {
  4. for {
  5. ch <- 1
  6. }
  7. }()
  8. return ch
  9. }
  10. func main() {
  11. for {
  12. select {
  13. case i := <- events():
  14. println("i=", i)
  15. }
  16. }
  17. }

We using a common generator pattern here, and select also is quite normal case, the problem at here is select would call events not just once! This loop would create new channel for every case statement! And leaving infinite go-routine that nobody care!

To avoid the problem, you have to using range:

  1. func main() {
  2. for i := range events() {
  3. // ...
  4. }
  5. }

But if you want to stop this looping, which means you still need to use select, then store the channel to other place is required. There are many ways to do that:

  • In structure:
  1. type eventGenerator struct {
  2. eventCh chan int
  3. ctx context.Context
  4. cancel context.CancelFunc
  5. }
  6. func NewEventGenerator(ctx context.Context) *eventGenerator {
  7. // better to get context from others place, even this is a most up level controller
  8. // because you can use `context.Background()` as argument if this is the most up level one
  9. ctx, cancel := context.WithCancel(ctx)
  10. return &eventGenerator{
  11. // don't forget to `make` a channel,
  12. // if you skip it, Go won't give you any warning
  13. // And anything you try to send to it would be ignored!
  14. // No Warning!
  15. eventCh: make(chan int),
  16. ctx: ctx,
  17. cancel: cancel,
  18. }
  19. }
  20. func (e *eventGenerator) Start() {
  21. go func() {
  22. defer close(e.eventCh)
  23. for {
  24. select {
  25. case _, closed := <- e.ctx.Done():
  26. if closed {
  27. return
  28. }
  29. default:
  30. e.eventCh <- 1
  31. }
  32. }
  33. }()
  34. }
  35. func (e *eventGenerator) Events() <-chan int { return e.eventCh }
  36. func (e *eventGenerator) Close() { e.cancel() }

Now you can write case <-eg.Events(): as you want after calling eg.Start() and stop it by eg.Close()

  • generator with outside channel
  1. func genEvents(ch chan int) {
  2. go func() {
  3. for {
  4. ch <- 1
  5. }
  6. }()
  7. }
  8. func main() {
  9. d := time.Now().Add(50 * time.Millisecond)
  10. ctx, cancel := context.WithDeadline(ctx, d)
  11. defer cancel()
  12. ch := make(chan int)
  13. genEvents(ch)
  14. for {
  15. select {
  16. case i := <-ch:
  17. println("i=", i)
  18. case <-ctx.Done():
  19. println("main:", ctx.Err().Error())
  20. close(ch)
  21. return
  22. }
  23. }
  24. }

2 misuse context.Done()

Let’s assuming there is a epoll like function call recv(), you would get something from it and deal with it, but it’s not based on channel, which means you can’t use it as case of select, how to deal with it?

  1. func handlingRecv(ctx context.Context) <-chan interface{} {
  2. ch := make(chan interface{})
  3. go func() {
  4. defer close(ch)
  5. for {
  6. data := recv()
  7. var v interface{}
  8. err := json.Unmarshal(data, v)
  9. // ignore error handing
  10. ch <- v
  11. select {
  12. case _, closed := <-ctx.Done():
  13. return
  14. }
  15. }
  16. }()
  17. return ch
  18. }

Code looks good? No, in fact the select would be blocked until this context be canceled, which means you can only get one message from recv(), and no warning, looks like a NICE networking problem, but it’s a bug of code actually.

This bug is easy to fix, in fact, easier than previous one a lot.

  1. select {
  2. case _, closed := <-ctx.Done():
  3. return
  4. default:
  5. // move job to here
  6. }

So easy, we just based on the fact, if no case in, it would do default block work

Conculsion

The bug show here might be is not hard to solve, but since everything could go wrong would go wrong, I still wrote it done and if it’s helpful that would be so great. Thanks for read.

ft_authoradmin  ft_create_time2019-06-08 18:51
 ft_update_time2019-06-08 18:51