CODEWITHSUNDEEP
go
sumit
sumit

Reputation: 37

Is this the correct way of using channels in go?

Here I am trying to iterate over res and launch a goroutine for every item. Inside every goroutine, again I am launching 3 goroutines in a buffered channel.

Running this code blocks the completion and does not allow the programme to complete. 

func (aui *AssignmentUtilImpl) MapAssignmentSubmissionData(res []AssignmentSubmissionNode) []AssignmentSubmission {

    if res == nil {
        return nil
    }

    submissions := []AssignmentSubmission{}

    ch := make(chan string, len(res))

    // map data
    for _, val := range res {

        go func(val AssignmentSubmissionNode) {
            sub := AssignmentSubmission{}
            c := make(chan string, 3)

            go mapSubmission(&sub, val, c)
            go mapUser(&sub, val, c)
            go mapFiles(&sub, val, c)

            sub.AssignmentId = val.AssignmentId
            sub.ClassroomId = val.ClassroomId

            for l := range c {
                fmt.Println(l)
            }

            close(c)

            submissions = append(submissions, sub)

            ch <- "submission2: " + sub.Id
        }(val)
    }

    for l := range ch {
        fmt.Println(l)
    }

    close(ch)

    return submissions
}

func mapFiles(sub *AssignmentSubmission, val AssignmentSubmissionNode, c chan string) {
    for _, f := range val.Files {
        file := resourceModule.File{}
        mapstructure.Decode(f.Data, &file)

        sub.Files = append(sub.Files, file)
    }
    c <- fmt.Sprintf("files: %d", len(sub.Files))
}

func mapUser(sub *AssignmentSubmission, val AssignmentSubmissionNode, c chan string) {
    user := userModule.User{}
    mapstructure.Decode(val.User.Data, &user)
    sub.User = user

    c <- "user: " + user.Id
}

func mapSubmission(sub *AssignmentSubmission, val AssignmentSubmissionNode, c chan string) {
    mapstructure.Decode(val.Submission.Data, &sub)

    c <- "submission1: " + sub.Id
}

Upvotes: 1

Views: 77

Answers (1)

jrefior
jrefior

Reputation: 4421

Move your close(c) and close(ch) to before the for ... range loops.

Once a buffered, not-closed channel reaches len(ch) == 0, a for e := range ch { ... } will block forever -- waiting for another goroutine to execute a send statement to the channel. Closing indicates that there will be no more elements sent to the channel (and any sends to a closed channel will cause a panic), and will cause the for e := range ch {...} loop to end when the channel is empty.

It gives following error panic: send on closed channel

Sending to a closed channel causes a panic. You're receiving this error because your main goroutine reached the close statement prior to a send in another goroutine.

You have multiple options for how to deal with this. One is to use sync.WaitGroup to wait for all goroutines that will send to a channel to finish before closing the channel. Something like:

go mapSubmission(&sub, val, c)
go mapUser(&sub, val, c)
go mapFiles(&sub, val, c)
// ...
wg.Wait()
close(c)
for element := range c {
    // ...

Another is to track the number of sends you expect on the channel, remove the for e := range ch {...} loops and replace them with a loop that will execute the receive operator on the channel the correct number of times. In this case you could also use unbuffered channels instead of a buffered channel if you wanted to. If you know how many times to call the receive operator, you don't need a for e := range ch {...}, and there's no need to close the channel.

Another approach is to not use a channel at all. Since all you're doing is printing to stdout, you could just move the prints to inside the goroutines, and use a sync.WaitGroup to ensure your main goroutine does not exit until your function goroutines have printed their output.

Upvotes: 1

Related Questions