1

I'm trying to implement an Observer Pattern suggested here; Observer pattern in Go language

(the code listed above doesn't compile and is incomplete). Here, is a complete code that compiles but I get deadlock error.

package main

import (
    "fmt"
)

type Publisher struct{
    listeners []chan int
}

type Subscriber struct{
    Channel chan int
    Name string
}

func (p *Publisher) Sub(c chan int){
    p.listeners = append(p.listeners, c)
}

func (p *Publisher) Pub(m int, quit chan int){
    for _, c := range p.listeners{
        c <- m
    }
    quit <- 0
}

func (s *Subscriber) ListenOnChannel(){
    data := <-s.Channel
    fmt.Printf("Name: %v; Data: %v\n", s.Name, data)            
}

func main() {
    quit := make(chan int)
    p := &Publisher{}
    subscribers := []*Subscriber{&Subscriber{Channel: make(chan int), Name: "1"}, &Subscriber{Channel: make(chan int), Name: "2"}, &Subscriber{Channel: make(chan int), Name: "3"}}
    for _, v := range subscribers{
        p.Sub(v.Channel)
        go v.ListenOnChannel() 
    }

    p.Pub(2, quit)

    <-quit              
}

Also, if I get rid of 'quit' completely, I get no error but it only prints first record.

Community
  • 1
  • 1
ashokgelal
  • 80,002
  • 26
  • 71
  • 84

2 Answers2

5

The problem is that you're sending to quit on the same goroutine that's receiving from quit.

quit has a buffer size of 0, which means that in order to proceed there has to be a sender on one side and a receiver on the other at the same time. You're sending, but no one's on the other end, so you wait forever. In this particular case the Go runtime is able to detect the problem and panic.

The reason only the first value is printed when you remove quit is that your main goroutine is exiting before your remaining two are able to print.

Do not just increase channel buffer sizes to get rid of problems like this. It can help (although in this case it doesn't), but it only covers up the problem and doesn't truly fix the underlying cause. Increasing a channel's buffer size is strictly an optimization. In fact, it's usually better to develop with no buffer because it makes concurrency problems more obvious.

There are two ways to fix the problem:

  • Keep quit, but send 0 on it in each goroutine inside ListenOnChannel. In main, make sure you receive a value from each goroutine before moving on. (In this case, you'll wait for three values.)
  • Use a WaitGroup. There's a good example of how it works in the documentation.
Evan Shaw
  • 23,839
  • 7
  • 70
  • 61
2

In general this looks good, but there is one problem. Remember that channels are either buffered or unbuffered (synchronous or asynchronous). When you send to an unbuffered channel or to a channel with a full buffer the sender will block until the data has been removed from the channel by a receiver.

So with that, I'll ask a question or two of my own:

  1. Is the quit channel synchronous or asynchronous?
  2. What happens in Pub when execution hits quit<-0?

One solution that fixes your problem and allows the code to run is to change the second-to-last code line to be go p.Pub(2, quit). But there is another solution. Can you see what it is?

I don't actually get the same behavior you do if I remove <-quit from the original code. And this should not affect the output because as it is written that line is never executed.

robothor
  • 600
  • 4
  • 6
  • Sorry but you got me more confused with your questions. And I can assure you that on my machine I get only one line printed if I remove quit completely. – ashokgelal Nov 09 '11 at 18:59
  • 1
    Check this out on the playground: http://play.golang.org/p/42tut8OKwG The point of the questions is to try to help _you_ figure out the problem. The other answer is good, but I think the core issue is important here. To answer the questions: 1. `quit` is synchronous because is was allocated via `make(chan int)`. 2. This makes it synchronous meaning that the main thread will _block_ when calling `quit <- 0`. – robothor Nov 09 '11 at 23:11
  • 1
    This means that the main thread will _never_ reach the `<-quit` line. The behavior that you are seeing is exposing a race condition or a bug and depends on which goroutine is scheduled by the runtime. http://play.golang.org/p/7Dv9sA6xv4 fixes your issue, but I think it is important that we understand _why_... – robothor Nov 09 '11 at 23:15
  • I appreciate you trying to help me understand the core issue. Sorry, if my comment sounded a bit rude; but it wasn't intended. Also, I'm talking about removing the 'quit' so that it has no effect overall (see this: http://play.golang.org/p/1gX50Mlhc7). Other thing, I figured out that using go p.Pub(...) would fix the issue but what if I don't want to, for some reasons, run p.Pub(...) as a goroutine but on the main thread? – ashokgelal Nov 10 '11 at 17:49
  • Now when we are here, I'm wondering what is an idiomatic way of implementing concurrent programming in go? I'm aware of official examples on concurrency but they don't talk about an idiomatic way of handling concurrency (I might be missing something here though). – ashokgelal Nov 10 '11 at 17:50
  • 1
    Ah, no worries. To answer the second question first... the issue is simply that the program is terminating before the other goroutines are scheduled and run. If you do `runtime.Gosched()` you'll see the expected result: This behavior is just an artifact of the implementation -- it could very well change with the scheduler. As to the first question -- remember that these channels are bidirectional. What about waiting for an acknowledgement from the channel? So you publish and wait for a response: ? – robothor Nov 10 '11 at 21:23
  • 1
    But in terms of idioms... if you are asking about when you want to choose to use a goroutine over some other construct, then I don't really have a good answer. It is always a design choice, but in general if you can think of two pieces of code as mostly independent then a concurrent approach is probably pretty reasonable. E.g. "Here, have this data, let me know when something interesting happens". It is interesting to take a look at and read Hoare's paper about Communicating Sequential Processes. – robothor Nov 10 '11 at 21:37
  • http://play.golang.org/p/m99A0OQuHH is what I was looking for. I completely ignored the fact that each subscriber can send notify back on the same channel to indicate that it is done with its task. I was thinking to use another set of channels for just this purpose (or use a WaitGroup as suggested by Even Shaw). Thank you so much. Thinking to bug you more if I get into any prob. Is there other ways I can contact you? This is great! I'm glad that I asked this question here. I wish I could accept both answers. But I'm thinking of blogging this and will give you the credits for making it clearer. – ashokgelal Nov 10 '11 at 22:28
  • Good! Glad you got it! Feel free to send me an email with my username at gmail dot com. – robothor Nov 11 '11 at 09:47