routing: fix race condition when resuming payments

[Crypt-iQ]

Found while running the integration tests with -race. Reuse of err variable when resuming payments in goroutines.

==================
WARNING: DATA RACE
Write at 0x00c000a680f0 by goroutine 147:
  github.com/lightningnetwork/lnd/routing.(*ChannelRouter).Start.func1()
      /Users/nsa/go/src/github.com/lightningnetwork/lnd/routing/router.go:536 +0x251

Previous write at 0x00c000a680f0 by goroutine 143:
  github.com/lightningnetwork/lnd/routing.(*ChannelRouter).Start.func1()
      /Users/nsa/go/src/github.com/lightningnetwork/lnd/routing/router.go:536 +0x251

Goroutine 147 (running) created at:
  github.com/lightningnetwork/lnd/routing.(*ChannelRouter).Start()
      /Users/nsa/go/src/github.com/lightningnetwork/lnd/routing/router.go:521 +0x42c
  github.com/lightningnetwork/lnd.(*server).Start.func1()
      /Users/nsa/go/src/github.com/lightningnetwork/lnd/server.go:1283 +0xaa0
  sync.(*Once).doSlow()
      /usr/local/Cellar/go/1.13/libexec/src/sync/once.go:66 +0x100
  sync.(*Once).Do()
      /usr/local/Cellar/go/1.13/libexec/src/sync/once.go:57 +0x68
  github.com/lightningnetwork/lnd.(*server).Start()
      /Users/nsa/go/src/github.com/lightningnetwork/lnd/server.go:1207 +0x7d
  github.com/lightningnetwork/lnd.Main()
      /Users/nsa/go/src/github.com/lightningnetwork/lnd/lnd.go:624 +0x29d3
  main.main()
      /Users/nsa/go/src/github.com/lightningnetwork/lnd/cmd/lnd/main.go:14 +0x3f

Goroutine 143 (running) created at:
  github.com/lightningnetwork/lnd/routing.(*ChannelRouter).Start()
      /Users/nsa/go/src/github.com/lightningnetwork/lnd/routing/router.go:521 +0x42c
  github.com/lightningnetwork/lnd.(*server).Start.func1()
      /Users/nsa/go/src/github.com/lightningnetwork/lnd/server.go:1283 +0xaa0
  sync.(*Once).doSlow()
      /usr/local/Cellar/go/1.13/libexec/src/sync/once.go:66 +0x100
  sync.(*Once).Do()
      /usr/local/Cellar/go/1.13/libexec/src/sync/once.go:57 +0x68
  github.com/lightningnetwork/lnd.(*server).Start()
      /Users/nsa/go/src/github.com/lightningnetwork/lnd/server.go:1207 +0x7d
  github.com/lightningnetwork/lnd.Main()
      /Users/nsa/go/src/github.com/lightningnetwork/lnd/lnd.go:624 +0x29d3
  main.main()
      /Users/nsa/go/src/github.com/lightningnetwork/lnd/cmd/lnd/main.go:14 +0x3f
==================

[Roasbeef]

Found while running the integration tests with -race

So you mean you've modified the binaries that are executed to build with -race?

[Crypt-iQ]

Found while running the integration tests with -race

So you mean you've modified the binaries that are executed to build with -race?

Yeah I had to increase all of the timeouts as well since the race detector is super slow.

[Roasbeef]

Very cool, once we clean up the lnd package a bit cmd/lnd, we should be able to use that instead to runt the integration tests rather than spinning up a new binary. I'm working on a PoC of this direction for the main package. Amongst many other things, it would allow us to have the itest count toward cover coverage and also likely make the tests much faster as each node is now just a series of goroutines rather than entirely new go runtime instance.

[Roasbeef]

Veryyyyy subtle....great find!

LGTM 👑

[Crypt-iQ]

Very cool, once we clean up the lnd package a bit cmd/lnd, we should be able to use that instead to runt the integration tests rather than spinning up a new binary. I'm working on a PoC of this direction for the main package. Amongst many other things, it would allow us to have the itest count toward cover coverage and also likely make the tests much faster as each node is now just a series of goroutines rather than entirely new go runtime instance.

that's great! I wanted to target the itests since they test more cases with various subsystems than the unit tests do. If this could be eventually run as part of the CI I'm sure it would catch even more bugs