13

I tried to find a similar question but I couldn't, so here I am asking:


I'm using close(ch) inside a recursive function. I need to close channel to terminate a range loop. However, since the function is recursive, the close is run multiple times which gives me:

panic: close of closed channel

If I comment-out the close(ch) statement, I receive:

fatal error: all goroutines are asleep - deadlock!

Please find below my code:

package main

import "golang.org/x/tour/tree"
import "fmt"

// Walk walks the tree t sending all values
// from the tree to the channel ch.
func Walk(t *tree.Tree, ch chan int) {
    ch<-(t.Value)
    if t.Left != nil { Walk(t.Left, ch) }
    if t.Right != nil { Walk(t.Right, ch) }
    close(ch) // => How to close a channel inside recursive function? ***
    return
}

func main() {
    ch := make(chan int)
    go Walk(tree.New(1), ch)
    for i := range ch {
        fmt.Print(i, " ")
    }
}
2
  • 1
    Closing a shared channel inside a recursive function is a fundamental design flaw.
    – Adrian
    Apr 19, 2018 at 15:49
  • @Adrian That's right. I implemented the solution suggested by the accepted answer.
    – Megidd
    Apr 19, 2018 at 16:01

3 Answers 3

26

Close the channel outside of the recursive function:

func Walk(t *tree.Tree, ch chan int) {
    ch<-(t.Value)
    if t.Left != nil { Walk(t.Left, ch) }
    if t.Right != nil { Walk(t.Right, ch) }
}

func main() {
    ch := make(chan int)
    go func() {
        defer close(ch)
        Walk(tree.New(1), ch)
    }()
    for i := range ch {
        fmt.Print(i, " ")
    }
}

Edit: This answer follows a common Go idiom of using defer for cleanup actions. As comments have noted, the defer is not necessary. The body of the anonymous function can also be written as:

        Walk(tree.New(1), ch)
        close(ch)
9
  • 1
    Isn't is against the advice that "Only the sender should close a channel, never the receiver." ? (From A Tour of Go, when it talks about Range and Close) Feb 26, 2020 at 14:01
  • 2
    @MichelePiccolini The answer follows the advice. The sending goroutine closes the channel, not the receiving goroutine. Feb 26, 2020 at 14:42
  • You are perfectly right. I was thinking about Walk being the sender, while we might consider the sender to be the whole lambda that calls Walk and defers close(ch). Stupid question, I'm sorry. Feb 26, 2020 at 15:04
  • @MichelePiccolini The advice refers to goroutines, not functions. How the code executed by the goroutines is split into functions is not relevant to the advice. Feb 26, 2020 at 15:17
  • 1
    Yes you are right. My confusion was just due to the fact that I was coming from trying to solve the esercise with Walk as the only thing spawned with go. Therefore in my mind I had "Walk = goroutine". Feb 26, 2020 at 15:32
3

Cerise Limón's answer works fine. I just wanted to mention that it's not mandatory to use defer inside the closure:

package main

import (
    "fmt"
    "golang.org/x/tour/tree"
)

func Walk(t *tree.Tree, ch chan int) {
    if t.Left != nil { Walk(t.Left, ch) }
    ch <- t.Value
    if t.Right != nil { Walk(t.Right, ch) }
}

func main() {
    ch := make(chan int)

    go func() {
        Walk(tree.New(1), ch)
        close(ch)
    }()

    for i := range ch {
        fmt.Print(i, " ")
    }
}

After the Walk function finishes, the channel is closed, and both actions are performed in a separated goroutine.

If we use

go Walk(tree.New(1), ch)
close(ch)

We are starting a goroutine but closing the channel immediately in the current one.


Since I wanted to close the channel inside the recursive function, I ended up doing this:

func Walk(t *tree.Tree, ch chan int, closeAtTheEnd bool) {
    if t.Left != nil {
        // fmt.Println("Walk the Left of", t.Value)
        Walk(t.Left, ch, false)
    } 

    // fmt.Println("Send to the channel:", t.Value)
    ch <- t.Value

    if t.Right != nil {
        // fmt.Println("Walk the Right of", t.Value)
        Walk(t.Right, ch, false)
    } 

    if closeAtTheEnd {
        close(ch)
    }
}

That way, recursive calls won't close the channel, and only the first call of Walk is allowed to do that (the tree starts at the root node and closes the channel after walking both directions).

func useWalkAndPrintTreeValues(k int) {
    c := make(chan int, 10)

    go Walk(tree.New(k), c, true)

    for x := range c {
        fmt.Print(x, " ")   
    }
}
0

I find this way of writing it to be much cleaner

func Walk(t *tree.Tree, ch chan int){
    WalkRecurse(t, ch)
    close(ch)
}
func WalkRecurse(t *tree.Tree, ch chan int){
    if t.Left != nil {
        WalkRecurse(t.Left, ch)
    }
    ch <- t.Value
    if t.Right != nil {
        WalkRecurse(t.Right, ch)
    }
}
func main(){
   ch := make(chan int)
   go Walk(tree.New(1), ch)
   for i := range ch {
      fmt.Println(i)
   }
}

Your Answer

Reminder: Answers generated by Artificial Intelligence tools are not allowed on Stack Overflow. Learn more

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.