CODEWITHSUNDEEP
godeadlockchannel
wwts17
wwts17

Reputation: 1

How to resolve Go channel deadlock?

I am learning go programming language, recently I have a problem, that I try much ways to run my code but I cannot correctly run. How can I change my program to do it?

package main

import (
    "fmt"
    "sync"
)

type Task struct {
    Id       int
    Callback chan int
}

func main() {
    var wg sync.WaitGroup
    subTask := make([]Task, 100)
    for i := 0; i < 100; i++ {
        go func(i int) {
            task := Task{
                Id:       i,
                Callback: make(chan int, 1),
            }
            task.Callback <- i
            subTask = append(subTask, task)
        }(i)
    }

    for _, v := range subTask {
        wg.Add(1)
        go func(v Task) {
            defer wg.Done()
            x := <-v.Callback
            fmt.Printf("%d ", x)
        }(v)
    }
    wg.Wait()
}

Upvotes: 0

Views: 289

Answers (4)

AJR
AJR

Reputation: 1661

Instead of a slice of tasks you might consider a chan of tasks.

I think this preserves your original idea of creating 100 channels written and read independently. It also avoids the data race.

func main() {
    subTasks := make(chan Task)
    var wg sync.WaitGroup
    for i := 0; i < 100; i++ {
        wg.Add(1)
        go func(i int) {
            defer wg.Done()
            task := Task{i, make(chan int)}
            subTasks <- task
            task.Callback <- i
        }(i)
    }

    go func() {
        wg.Wait()
        close(subTasks)
    }()

    for v := range subTasks {
        go func(v Task) {
            fmt.Println(<-v.Callback)
        }(v)
    }
}

Run in the playground

Upvotes: 0

AJR
AJR

Reputation: 1661

One problem is that you are appending to the slice instead of updating the existing slice items. Also you don't need a buffered chan.

func main() {
    subTask := make([]Task, 100)
    for i := range subTask {
        go func(i int) {
            subTask[i] = Task{i, make(chan int)}
            subTask[i].Callback <- i
        }(i)
    }

    var wg sync.WaitGroup
    wg.Add(len(subTask))
    for _, v := range subTask {
        go func(v Task) {
            defer wg.Done()
            fmt.Println(<-v.Callback)
        }(v)
    }
    wg.Wait()
}

Upvotes: 0

Thundercat
Thundercat

Reputation: 120941

There's a data race on subTask. The task initialization goroutines read and write the variable subTask with no synchronization.

The intent of the program is to create and initialize a slice of 100 Task values, but it creates a slice with 100 zero value Tasks and appends 100 more initialized Tasks (ignoring the data race issue just mentioned).

Fix both of these issues by assigning tasks to the slice elements:

for i := 0; i < 100; i++ {
    go func(i int) {
        task := Task{
            Id:       i,
            Callback: make(chan int, 1),
        }
        task.Callback <- i
        subTask[i] = task
    }(i)
}

There's a data race on the subTask elements. There's no guarantee that the task initialization goroutines complete writing to the elements before the main goroutine ranges over those those elements. Fix by using a wait group to coordinate the completion of the initialization goroutines and the main goroutine:

subTask := make([]Task, 100)
for i := 0; i < 100; i++ {
    wg.Add(1)
    go func(i int) {
        task := Task{
            Id:       i,
            Callback: make(chan int, 1),
        }
        task.Callback <- i
        subTask[i] = task
        wg.Done()
    }(i)
}
wg.Wait()

Run the code on the playground.

The race detector reports both of the data races mentioned above.

If the code in the question is the actual code and not a minimal example for the purpose of asking the question, then the goroutines are not needed at all.

Upvotes: 1

Ravi Chandak
Ravi Chandak

Reputation: 113

If a channel is nil the <-c receiving from c blocks forever. hence the deadlock The reason the channel could be nil is that one of the goroutines from the 1st for loop may not have been executed at the time the goroutine receiving was executed.

So your code would work fine if you assume that all the goroutines from the 1st for loop are executed before the 2nd for loop starts.

Adding in a sleep can show you the difference but you should actually tackle is issue.

One more thing that could be the issue is subTask := make([]Task, 100) This statement creates 100 empty task obj in the slice and append adds more to it so the length grows to 200 eventually.

https://play.golang.org/p/4bZDJ2zvKdF

Upvotes: 0

Related Questions