From: Rob Pike <r@golang.org>
Date: Wed, 2 Oct 2013 18:35:25 +0000 (-0700)
Subject: doc/effective_go: fix server example that shares var between goroutines
X-Git-Tag: go1.2rc2~89
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=d38ed2a9f224dbc79adbed4fe49fe2aef137bf5b;p=gostls13.git

doc/effective_go: fix server example that shares var between goroutines
Use it as a teaching example about how to solve this problem.

Fixes #6501

R=golang-dev, adg, rsc
CC=golang-dev
https://golang.org/cl/14250043
---

diff --git a/doc/effective_go.html b/doc/effective_go.html
index 35b15e8df5..6e7ee1a28e 100644
--- a/doc/effective_go.html
+++ b/doc/effective_go.html
@@ -2981,12 +2981,53 @@ of them can run at any moment.
 As a result, the program can consume unlimited resources if the requests come in too fast.
 We can address that deficiency by changing <code>Serve</code> to
 gate the creation of the goroutines.
+Here's an obvious solution, but beware it has a bug we'll fix subsequently:
 </p>
 
 <pre>
 func Serve(queue chan *Request) {
     for req := range queue {
         &lt;-sem
+        go func() {
+            process(req) // Buggy; see explanation below.
+            sem &lt;- 1
+        }()
+    }
+}</pre>
+
+<p>
+The bug is that in a Go <code>for</code> loop, the loop variable
+is reused for each iteration, so the <code>req</code>
+variable is shared across all goroutines.
+That's not what we want.
+We need to make sure that <code>req</code> is unique for each goroutine.
+Here's one way to do that, passing the value of <code>req</code> as an argument
+to the closure in the goroutine:
+</p>
+
+<pre>
+func Serve(queue chan *Request) {
+    for req := range queue {
+        &lt;-sem
+        go func(req *Request) {
+            process(req)
+            sem &lt;- 1
+        }(req)
+    }
+}</pre>
+
+<p>
+Compare this version with the previous to see the difference in how
+the closure is declared and run.
+Another solution is just to create a new variable with the same
+name, as in this example:
+</p>
+
+<pre>
+func Serve(queue chan *Request) {
+    for req := range queue {
+        &lt;-sem
+        req := req // Create new instance of req for the goroutine.
         go func() {
             process(req)
             sem &lt;- 1
@@ -2995,7 +3036,22 @@ func Serve(queue chan *Request) {
 }</pre>
 
 <p>
-Another solution that manages resources well is to start a fixed
+It may seem odd to write
+</p>
+
+<pre>
+req := req
+</pre>
+
+<p>
+but it's a legal and idiomatic in Go to do this.
+You get a fresh version of the variable with the same name, deliberately
+shadowing the loop variable locally but unique to each goroutine.
+</p>
+
+<p>
+Going back to the general problem of writing the server,
+another approach that manages resources well is to start a fixed
 number of <code>handle</code> goroutines all reading from the request
 channel.
 The number of goroutines limits the number of simultaneous