CODEWITHSUNDEEP
go
Jakob Svenningsson
Jakob Svenningsson

Reputation: 4315

Bug detect, go channels with select

There is supposed to be a bug in this bit of code. My mate told me that it contains a memory leak and it occurs when the time out case happens in the select statement before the go function has finished and he also told me that adding a buffer of size one to ch would solve the problem. But i have a hard time understanding why it would solve the problem and would appreciate if someone could explain if for me? I've tried to search for the answer myself but with no success.

Thanks.

func Read(url string, timeout time.Duration) (res *Response) {
    ch := make(chan *Response)
    go func() {
        time.Sleep(time.Millisecond * 300)
        ch <- Get(url)
    }()
    select {
    case res = <-ch:
    case <-time.After(timeout):
        res = &Response{"Gateway timeout\n", 504}
    }
}

Upvotes: 1

Views: 186

Answers (1)

Tobia
Tobia

Reputation: 18811

The fact is that a channel without a buffer, called synchronous, will block both the sender and the receiver until they can complete their exchange.

Much like if you had to hand over something to your mate and you both knew the meeting place, but not the time. The first one to get there will wait for the other, whether it's the sender or the receiver. Now, given that computers are stupid :-) if one of them forgets about the appointment, the other will verily wait forever.

The specific bug here is that when the select chooses the time.After (that is, the timeout occurs) nobody will be there to receive from <-ch anymore. Not ever. So the poor go func() will sit there forever, waiting for someone to take her *Response, but nobody will ever show up.

This does not actually waste any CPU power, but it wastes memory: that needed to keep track of the channel, of the goroutine, of her stack—however small—and her local variables. The memory will never be reclaimed until the entire process terminates or is killed.

In a server, serving a lot of clients, this will build up quickly until the application eats all the memory of the server and—if you're lucky—gets killed by the OS safety measures, without taking down the entire machine.

Using a buffered channel is one way to solve the issue, because then, whenever the poor go func() has her *Response ready, she will be able to store it into the channel's buffer, even if nobody is there to get it, and terminate peacefully. As soon as that happens, Go's garbage collector will notice that no live goroutine is holding any pointers to that channel anymore, so it will collect the channel and the *Response it points to and recycle all those bytes.

Upvotes: 2

Related Questions