Golang 并发代码审查

Question

我试图了解Golang并发的最佳实践。我读了O'Reilly关于Go并发的书，然后回到了Golang Codewalks，特别是这个例子：https://golang.org/doc/codewalk/sharemem/这是我希望与您一起回顾的代码，以便更多地了解Go。我的第一印象是，这段代码正在破坏一些最佳实践。这当然是我（非常）没有经验的观点，我想讨论并获得对这个过程的一些见解。这不是关于谁对谁错，请好心，我只是想分享我的观点并获得一些反馈。也许这个讨论会帮助其他人理解我为什么错了，并教他们一些东西。我完全知道这个代码的目的是教初学者，而不是成为完美的代码。问题 1 - 无 Goroutine 清理逻辑func main() {    // Create our input and output channels.    pending, complete := make(chan *Resource), make(chan *Resource)    // Launch the StateMonitor.    status := StateMonitor(statusInterval)    // Launch some Poller goroutines.    for i := 0; i < numPollers; i++ {        go Poller(pending, complete, status)    }    // Send some Resources to the pending queue.    go func() {        for _, url := range urls {            pending <- &Resource{url: url}        }    }()    for r := range complete {        go r.Sleep(pending)    }}主要方法没有办法清理Goroutines，这意味着如果这是库的一部分，它们将被泄露。问题 2 - 编写器未生成频道我认为，作为最佳实践，创建、编写和清理通道的逻辑应由单个实体（或一组实体）控制。这背后的原因是，作者在写入封闭频道时会感到恐慌。因此，编写器最好创建通道，写入该通道并控制何时应关闭该通道。如果有多个写入器，则可以将它们与等待组同步。func StateMonitor(updateInterval time.Duration) chan<- State {    updates := make(chan State)    urlStatus := make(map[string]string)    ticker := time.NewTicker(updateInterval)    go func() {        for {            select {            case <-ticker.C:                logState(urlStatus)            case s := <-updates:                urlStatus[s.url] = s.status            }        }    }()    return updates}此函数不应负责创建更新频道，因为它是频道的读取器，而不是编写器。此通道的编写者应创建它并将其传递给此函数。基本上对功能说“我会通过这个渠道将更新传递给你”。但相反，这个函数正在创建一个通道，目前还不清楚谁负责清理它。

Answer 1

1. 这是方法，因此无需清理。返回时，程序将退出。如果不是 ，那么你是对的。mainmainmain

2. 没有适合所有用例的最佳实践。您在此处显示的代码是一种非常常见的模式。该函数创建一个 goroutine，并返回一个通道，以便其他人可以与该 goroutine 进行通信。没有规则来控制必须如何创建通道。但是，没有办法终止该goroutine。这种模式非常适合的一个用例是从数据库中读取大型结果集。该通道允许在从数据库中读取数据时对其进行流式处理。在这种情况下，通常还有其他方法可以终止 goroutine，例如传递上下文。

3. 同样，对于如何创建/关闭渠道没有硬性规定。通道可以保持打开状态，当它不再使用时，它将被垃圾回收。如果用例需要，通道可以无限期地保持打开状态，并且您担心的场景永远不会发生。

Answer 2

正如您询问此代码是否是库的一部分时，是的，在库函数内生成没有清理的goroutines将是糟糕的做法。如果这些 goroutine 执行了库的记录行为，那么调用方不知道该行为何时会发生是有问题的。如果您有任何通常“开火即忘记”的行为，则应该由呼叫者选择何时忘记它。例如：

func doAfter5Minutes(f func()) {

   go func() {

       time.Sleep(5 * time.Minute)

       f()

       log.Println("done!")

   }()

}

有道理，对吧？调用该函数时，它会在 5 分钟后执行某些操作。问题是很容易像这样误用这个函数：

// do the important task every 5 minutes

for {

    doAfter5Minutes(importantTaskFunction)

}

乍一看，这似乎很好。我们每5分钟做一次重要的任务，对吧？实际上，我们正在非常快速地生成许多goroutines，可能会在它们开始下降之前消耗所有可用的内存。

我们可以实现某种回调或通道，以便在任务完成时发出信号，但实际上，函数应该像这样简化：

func doAfter5Minutes(f func()) {

   time.Sleep(5 * time.Minute)

   f()

   log.Println("done!")

}

现在，调用方可以选择如何使用它：

// call synchronously

doAfter5Minutes(importantTaskFunction)

// fire and forget

go doAfter5Minutes(importantTaskFunction)

这个功能也可以说是应该改变的。正如你所说，作者应该有效地拥有这个频道，因为他们应该是关闭它的人。事实上，这个通道读取函数坚持创建它从中读取的通道，这实际上迫使自己陷入上面提到的这种可怜的“开火和忘记”模式。请注意函数需要如何从通道读取，但它也需要在读取之前返回通道。因此，它必须将阅读行为放在一个新的，不受管理的goroutine中，以允许自己立即返回通道。

func StateMonitor(updates chan State, updateInterval time.Duration) {

    urlStatus := make(map[string]string)

    ticker := time.NewTicker(updateInterval)

    defer ticker.Stop() // not stopping the ticker is also a resource leak

    for {

        select {

        case <-ticker.C:

            logState(urlStatus)

        case s := <-updates:

            urlStatus[s.url] = s.status

        }

    }

}

请注意，该函数现在更简单、更灵活、更同步。以前的版本真正完成的唯一一件事是，它（大多数）保证的每个实例都有一个通道，并且您不会遇到多个监视器在同一通道上竞争读取的情况。虽然这可以帮助您避免特定类别的错误，但它也使函数的灵活性大大降低，并且更有可能发生资源泄漏。StateMonitor

我不确定我是否真正理解了这个例子，但通道关闭的黄金法则是，编写者应该始终负责关闭通道。请记住此规则，并注意有关此代码的几点：

该方法写入Sleepr

该方法同时执行，没有跟踪运行实例数量、实例处于什么状态等的方法。Sleep

仅基于这些要点，我们可以说程序中可能没有任何地方可以安全关闭 ，因为似乎无法知道它是否会再次使用。r