Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't access loop variables in goroutines directly #7188

Merged
merged 3 commits into from
Nov 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions chainntnfs/bitcoindnotify/bitcoind.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,9 @@ out:
//
// TODO(wilmer): add retry logic if rescan fails?
b.wg.Add(1)
go func() {

//nolint:lll
go func(msg *chainntnfs.HistoricalConfDispatch) {
defer b.wg.Done()

confDetails, _, err := b.historicalConfDetails(
Expand Down Expand Up @@ -269,7 +271,7 @@ out:
"details of %v: %v",
msg.ConfRequest, err)
}
}()
}(msg)

case *chainntnfs.HistoricalSpendDispatch:
// In order to ensure we don't block the caller
Expand All @@ -278,7 +280,9 @@ out:
//
// TODO(wilmer): add retry logic if rescan fails?
b.wg.Add(1)
go func() {

//nolint:lll
go func(msg *chainntnfs.HistoricalSpendDispatch) {
defer b.wg.Done()

spendDetails, err := b.historicalSpendDetails(
Expand Down Expand Up @@ -320,7 +324,7 @@ out:
"details of %v: %v",
msg.SpendRequest, err)
}
}()
}(msg)

case *blockEpochRegistration:
chainntnfs.Log.Infof("New block epoch subscription")
Expand Down
6 changes: 4 additions & 2 deletions chainntnfs/btcdnotify/btcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,9 @@ out:
//
// TODO(wilmer): add retry logic if rescan fails?
b.wg.Add(1)
go func() {

//nolint:lll
go func(msg *chainntnfs.HistoricalConfDispatch) {
defer b.wg.Done()

confDetails, _, err := b.historicalConfDetails(
Expand All @@ -372,7 +374,7 @@ out:
if err != nil {
chainntnfs.Log.Error(err)
}
}()
}(msg)

case *blockEpochRegistration:
chainntnfs.Log.Infof("New block epoch subscription")
Expand Down
6 changes: 4 additions & 2 deletions chainntnfs/neutrinonotify/neutrino.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,9 @@ func (n *NeutrinoNotifier) notificationDispatcher() {
// chain asynchronously to prevent blocking
// potentially long rescans.
n.wg.Add(1)
go func() {

//nolint:lll
go func(msg *chainntnfs.HistoricalConfDispatch) {
defer n.wg.Done()

confDetails, err := n.historicalConfDetails(
Expand All @@ -457,7 +459,7 @@ func (n *NeutrinoNotifier) notificationDispatcher() {
if err != nil {
chainntnfs.Log.Error(err)
}
}()
}(msg)

case *blockEpochRegistration:
chainntnfs.Log.Infof("New block epoch subscription")
Expand Down
2 changes: 2 additions & 0 deletions discovery/gossiper.go
Original file line number Diff line number Diff line change
Expand Up @@ -1325,6 +1325,7 @@ func (d *AuthenticatedGossiper) networkHandler() {
d.wg.Add(1)
go func() {
defer d.wg.Done()

guggero marked this conversation as resolved.
Show resolved Hide resolved
log.Infof("Broadcasting %v new announcements in %d sub batches",
len(announcementBatch), len(splitAnnouncementBatch))

Expand Down Expand Up @@ -2947,6 +2948,7 @@ func (d *AuthenticatedGossiper) handleAnnSig(nMsg *networkMsg,
d.wg.Add(1)
go func() {
defer d.wg.Done()

log.Debugf("Received half proof for channel "+
"%v with existing full proof. Sending"+
" full proof to peer=%x",
Expand Down
3 changes: 3 additions & 0 deletions docs/release-notes/release-notes-0.16.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,9 @@ certain large transactions](https://github.com/lightningnetwork/lnd/pull/7100).

* [test: fix loop variables being accessed in
closures](https://github.com/lightningnetwork/lnd/pull/7032).

* [Fix loop and other temporary variables being accessed in
goroutines](https://github.com/lightningnetwork/lnd/pull/7188).

## Watchtowers

Expand Down
1 change: 1 addition & 0 deletions funding/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -1969,6 +1969,7 @@ func (f *Manager) handleFundingAccept(peer lnpeer.Peer,
f.wg.Add(1)
go func() {
defer f.wg.Done()

f.waitForPsbt(psbtIntent, resCtx, pendingChanID)
}()

Expand Down
5 changes: 3 additions & 2 deletions healthcheck/healthcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,11 @@ func (m *Monitor) Start() error {
}

m.wg.Add(1)
go func() {
go func(check *Observation) {
defer m.wg.Done()

check.monitor(m.cfg.Shutdown, m.quit)
}()
}(check)
}

return nil
Expand Down
1 change: 1 addition & 0 deletions htlcswitch/switch.go
Original file line number Diff line number Diff line change
Expand Up @@ -1742,6 +1742,7 @@ func (s *Switch) htlcForwarder() {
wg.Add(1)
go func(l ChannelLink) {
defer wg.Done()

l.Stop()
}(link)
}
Expand Down
8 changes: 4 additions & 4 deletions lntest/itest/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,12 @@ func completePaymentRequests(client lnrpc.LightningClient,
// Launch all payments simultaneously.
results := make(chan error)
for _, payReq := range paymentRequests {
payReqCopy := payReq
go func() {
err := send(payReqCopy)
go func(payReq string) {
err := send(payReq)
if awaitResponse {
results <- err
}
}()
}(payReq)
}

// If awaiting a response, verify that all payments succeeded.
Expand Down Expand Up @@ -406,6 +405,7 @@ func subscribeChannelNotifications(ctxb context.Context, t *harnessTest,
chanUpdates := make(chan *lnrpc.ChannelEventUpdate, 20)
go func() {
defer cancelFunc()

for {
select {
case <-quit:
Expand Down
14 changes: 9 additions & 5 deletions lnwallet/btcwallet/btcwallet.go
Original file line number Diff line number Diff line change
Expand Up @@ -1483,9 +1483,13 @@ out:

// Launch a goroutine to re-package and send
// notifications for any newly confirmed transactions.
go func() {
//nolint:lll
go func(txNtfn *wallet.TransactionNotifications) {
for _, block := range txNtfn.AttachedBlocks {
details, err := minedTransactionsToDetails(currentHeight, block, t.w.ChainParams())
details, err := minedTransactionsToDetails(
currentHeight, block,
t.w.ChainParams(),
)
if err != nil {
continue
}
Expand All @@ -1499,11 +1503,11 @@ out:
}
}

}()
}(txNtfn)

// Launch a goroutine to re-package and send
// notifications for any newly unconfirmed transactions.
go func() {
go func(txNtfn *wallet.TransactionNotifications) {
for _, tx := range txNtfn.UnminedTransactions {
detail, err := unminedTransactionsToDetail(
tx, t.w.ChainParams(),
Expand All @@ -1518,7 +1522,7 @@ out:
return
}
}
}()
}(txNtfn)
case <-t.quit:
break out
}
Expand Down
3 changes: 2 additions & 1 deletion rpcperms/middleware_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,9 @@ func (h *MiddlewareHandler) Run() error {
// request to the client).
h.wg.Add(1)
go func() {
defer h.wg.Done()

h.receiveResponses(errChan, responses)
h.wg.Done()
}()

return h.sendInterceptRequests(errChan, responses)
Expand Down
4 changes: 2 additions & 2 deletions rpcserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -5214,7 +5214,7 @@ sendLoop:
// payment so we can continue to serve requests while
// this payment is being dispatched.
wg.Add(1)
go func() {
go func(payIntent *rpcPaymentIntent) {
defer wg.Done()

// Attempt to grab a free semaphore slot, using
Expand Down Expand Up @@ -5292,7 +5292,7 @@ sendLoop:
}
return
}
}()
}(payIntent)
}
}

Expand Down
1 change: 1 addition & 0 deletions sweep/sweeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -1415,6 +1415,7 @@ func (s *UtxoSweeper) waitForSpend(outpoint wire.OutPoint,
s.wg.Add(1)
go func() {
defer s.wg.Done()

select {
case spend, ok := <-spendEvent.Spend:
if !ok {
Expand Down
1 change: 1 addition & 0 deletions ticker/force.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func NewForce(interval time.Duration) *Force {
m.wg.Add(1)
go func() {
defer m.wg.Done()

for {
select {
case t := <-m.ticker:
Expand Down
1 change: 1 addition & 0 deletions watchtower/wtclient/session_queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,7 @@ func (s *sessionQueueSet) ApplyAndWait(getApply func(*sessionQueue) func()) {
wg.Add(1)
go func(sq *sessionQueue) {
defer wg.Done()

getApply(sq)()
}(sessionq)
}
Expand Down