a common iteration usage pattern that causes sneaky goroutine leakage

269 views
Skip to first unread message

Petar Maymounkov

unread,
Jul 5, 2011, 7:19:43 PM7/5/11
to golang-nuts
I've discovered a sneaky pattern that is prone to errors.

It is now common practice to provide Iter() methods for
some containers, where ther Iter() returns a channel which
is about to receive the iteration sequence.

Such Iter() methods are usually implemented using the following code:

func (t *Container) Iter() <-chan *Item {
ch := make(chan *Item)
go func() {
for _, item := range [...] {
ch <- s
}
close(ch)
}()
return ch

}

The problem arises when you attempt to use an iterator channel, but
you don't
intend to go all the way to the end:

ch := container.Iter()
for item, ok := <-ch; ok; item, ok = <-ch {
if [...] {
break
}
}

If you break the loop prematurely, the anonymous go-routine hangs
forever.

This code does not interfere with the normal function of your program
in anyway,
other than leaving a zombie go-routine.

Consequently, your program runs out of memory.

When you inspect the memory profile, you don't see any problems,
because the
memory profile does not report space taken by go-routines.

It took me forever to remember to look at how many go-routines are
active
and to finally decipher why my program was running out of memory.

Comments? Better practices?

What concerns me here is that this example speaks to the limitations
of
channels as iterators. Note that the receiving end cannot close the
channel
as an indication of "I am done". (Only the sender can.)

--P

David Symonds

unread,
Jul 5, 2011, 7:23:33 PM7/5/11
to Petar Maymounkov, golang-nuts
On Wed, Jul 6, 2011 at 9:19 AM, Petar Maymounkov <pet...@gmail.com> wrote:

> Comments? Better practices?

You've just described one of the good reasons why the Iter/chan model
is, in fact, not common practice. Remember the exp/iterable package,
which is long gone?

You're probably better off either exposing the data as a slice (if
practical), or adding a Do method that takes a func to call for each
element in the container.


Dave.

Petar Maymounkov

unread,
Jul 5, 2011, 7:26:30 PM7/5/11
to David Symonds, golang-nuts
Good to know there is an agreement here. Just wanted to make sure
that we don't insist this is a way to write iterators, since I've seen this
pattern in various locations in the go source. Maybe they are not
around any more.

--P

Islan Dberry

unread,
Jul 5, 2011, 7:41:18 PM7/5/11
to golan...@googlegroups.com, David Symonds
since I've seen this pattern in various locations in the go source

I think there's only one remaining in expvar.

peterGo

unread,
Jul 5, 2011, 10:07:07 PM7/5/11
to golang-nuts
Petar,

Issue 1111 - go - list.Iter() leaks memory
http://code.google.com/p/go/issues/detail?id=1111

Issue 296 - go - goroutines blocked on channel send are not collected
http://code.google.com/p/go/issues/detail?id=296

Peter

Kyle Consalus

unread,
Jul 5, 2011, 10:27:56 PM7/5/11
to Petar Maymounkov, golang-nuts
Reply all
Reply to author
Forward
0 new messages