Fix a goroutine misusage in the implementation of Worker.Subscribe()

[AkihiroSuda]

Fix moby/moby#28915

Signed-off-by: Akihiro Suda suda.akihiro@lab.ntt.co.jp

[codecov-io]

Current coverage is 55.05% (diff: 0.00%)

Merging #1775 into master will decrease coverage by 0.03%

@@             master      #1775   diff @@
==========================================
  Files           102        102          
  Lines         16871      16871          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits           9294       9288     -6   
- Misses         6419       6428     +9   
+ Partials       1158       1155     -3   

Powered by Codecov. Last update 2422b48...35324fd

[thaJeztah]

ping @aluzzardi ptal

[LK4D4]

LGTM

[dongluochen]

LGTM

[aaronlehmann]

LGTM

[stevvooe]

Classic!

[AkihiroSuda]

BTW, is there any static code checking tool for this "common mistake"?

[aaronlehmann]

I don't know of any, but I think it would be a very good addition to go vet.

[stevvooe]

@AkihiroSuda @aaronlehmann Just don't do it. ;)

The best way to understand why this is to think of the for loop variables like this:

{ // enclosing block
  var tm *taskManager
  for tm = range taskManagers {
    // tm gets a new value on each iteration.
  }
}

Checkout https://play.golang.org/p/Bat8jtGgEh. It results in all goroutines printing last value of i. tm as a pointer works the same way. The way around this is to create new variable, on each loop, and let that get closed:

{ // enclosing block
  var tm *taskManager
  for tm = range taskManagers {
     tm := tm
    // tm gets a new value on each iteration.
  }
}

The above creates a new tm per loop scope, which can get closed over independently. The best way to think of this is that these variables are members of for loop's scope, not the scope of each run of the for loop (this makes a lot of sense of memory efficiency).

Really, this could be caught with the race detector.