Skip to content

Commit fe8784a

Browse files
RoasbeefRoasbeef
committed
channeldb: fix race in TestPackager by removing global test var
In this commit, we fix a race in the `TestPackager` series on channeldb. A few tests were sharing the same global variable of the set of log updates, which includes a pointer to an HTLC. The `ExtraData` value of the HTLC would then be mutated once we go to encode the message on disk. To fix this, we the global with a function that returns a new instance of all the test data. ``` ================== WARNING: DATA RACE Write at 0x0000021b0a48 by goroutine 2896: github.com/lightningnetwork/lnd/lnwire.(*ExtraOpaqueData).PackRecords() /home/runner/work/lnd/lnd/lnwire/extra_bytes.go:74 +0x546 github.com/lightningnetwork/lnd/lnwire.EncodeMessageExtraData() /home/runner/work/lnd/lnd/lnwire/extra_bytes.go:121 +0x4d github.com/lightningnetwork/lnd/lnwire.(*UpdateAddHTLC).Encode() /home/runner/work/lnd/lnd/lnwire/update_add_htlc.go:164 +0x5af github.com/lightningnetwork/lnd/lnwire.WriteMessage() /home/runner/work/lnd/lnd/lnwire/message.go:330 +0x351 github.com/lightningnetwork/lnd/channeldb.WriteElement() /home/runner/work/lnd/lnd/channeldb/codec.go:186 +0x1975 github.com/lightningnetwork/lnd/channeldb.WriteElements() /home/runner/work/lnd/lnd/channeldb/codec.go:247 +0x14f github.com/lightningnetwork/lnd/channeldb.serializeLogUpdate() /home/runner/work/lnd/lnd/channeldb/channel.go:2529 +0x3c github.com/lightningnetwork/lnd/channeldb.putLogUpdate() /home/runner/work/lnd/lnd/channeldb/forwarding_package.go:525 +0xae github.com/lightningnetwork/lnd/channeldb.(*ChannelPackager).AddFwdPkg() /home/runner/work/lnd/lnd/channeldb/forwarding_package.go:489 +0x684 github.com/lightningnetwork/lnd/channeldb_test.TestPackagerOnlyAdds.func1() /home/runner/work/lnd/lnd/channeldb/forwarding_package_test.go:283 +0x4c github.com/btcsuite/btcwallet/walletdb/bdb.(*db).Update() /home/runner/go/pkg/mod/github.com/btcsuite/btcwallet/[email protected]/bdb/db.go:429 +0xe5 github.com/lightningnetwork/lnd/kvdb.Update() /home/runner/go/pkg/mod/github.com/lightningnetwork/lnd/[email protected]/interface.go:16 +0x258 github.com/lightningnetwork/lnd/channeldb_test.TestPackagerOnlyAdds() /home/runner/work/lnd/lnd/channeldb/forwarding_package_test.go:282 +0x17b testing.tRunner() /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1595 +0x238 testing.(*T).Run.func1() /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1648 +0x44 Previous write at 0x0000021b0a48 by goroutine 2898: github.com/lightningnetwork/lnd/lnwire.(*ExtraOpaqueData).PackRecords() /home/runner/work/lnd/lnd/lnwire/extra_bytes.go:74 +0x546 github.com/lightningnetwork/lnd/lnwire.EncodeMessageExtraData() /home/runner/work/lnd/lnd/lnwire/extra_bytes.go:121 +0x4d github.com/lightningnetwork/lnd/lnwire.(*UpdateAddHTLC).Encode() /home/runner/work/lnd/lnd/lnwire/update_add_htlc.go:164 +0x5af github.com/lightningnetwork/lnd/lnwire.WriteMessage() /home/runner/work/lnd/lnd/lnwire/message.go:330 +0x351 github.com/lightningnetwork/lnd/channeldb.WriteElement() /home/runner/work/lnd/lnd/channeldb/codec.go:186 +0x1975 github.com/lightningnetwork/lnd/channeldb.WriteElements() /home/runner/work/lnd/lnd/channeldb/codec.go:247 +0x14f github.com/lightningnetwork/lnd/channeldb.serializeLogUpdate() /home/runner/work/lnd/lnd/channeldb/channel.go:2529 +0x3c github.com/lightningnetwork/lnd/channeldb.putLogUpdate() /home/runner/work/lnd/lnd/channeldb/forwarding_package.go:525 +0xae github.com/lightningnetwork/lnd/channeldb.(*ChannelPackager).AddFwdPkg() /home/runner/work/lnd/lnd/channeldb/forwarding_package.go:489 +0x684 github.com/lightningnetwork/lnd/channeldb_test.TestPackagerAddsThenSettleFails.func1() /home/runner/work/lnd/lnd/channeldb/forwarding_package_test.go:490 +0x4c github.com/btcsuite/btcwallet/walletdb/bdb.(*db).Update() /home/runner/go/pkg/mod/github.com/btcsuite/btcwallet/[email protected]/bdb/db.go:429 +0xe5 github.com/lightningnetwork/lnd/kvdb.Update() /home/runner/go/pkg/mod/github.com/lightningnetwork/lnd/[email protected]/interface.go:16 +0x2cd github.com/lightningnetwork/lnd/channeldb_test.TestPackagerAddsThenSettleFails() /home/runner/work/lnd/lnd/channeldb/forwarding_package_test.go:489 +0x1e7 testing.tRunner() /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1595 +0x238 testing.(*T).Run.func1() /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1648 +0x44 Goroutine 2896 (running) created at: testing.(*T).Run() /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1648 +0x82a testing.runTests.func1() /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:2054 +0x84 testing.tRunner() /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1595 +0x238 testing.runTests() /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:2052 +0x896 testing.(*M).Run() /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1925 +0xb57 github.com/lightningnetwork/lnd/kvdb.RunTests() /home/runner/go/pkg/mod/github.com/lightningnetwork/lnd/[email protected]/test_utils.go:23 +0x26 github.com/lightningnetwork/lnd/channeldb.TestMain() /home/runner/work/lnd/lnd/channeldb/setup_test.go:10 +0x308 main.main() _testmain.go:321 +0x303 Goroutine 2898 (running) created at: testing.(*T).Run() /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1648 +0x82a testing.runTests.func1() /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:2054 +0x84 testing.tRunner() /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1595 +0x238 testing.runTests() /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:2052 +0x896 testing.(*M).Run() /home/runner/go/pkg/mod/golang.org/[email protected]/src/testing/testing.go:1925 +0xb57 github.com/lightningnetwork/lnd/kvdb.RunTests() /home/runner/go/pkg/mod/github.com/lightningnetwork/lnd/[email protected]/test_utils.go:23 +0x26 github.com/lightningnetwork/lnd/channeldb.TestMain() /home/runner/work/lnd/lnd/channeldb/setup_test.go:10 +0x308 main.main() _testmain.go:321 +0x303 ================== ```
1 parent e6d6789 commit fe8784a

File tree

1 file changed

+34
-21
lines changed

1 file changed

+34
-21
lines changed

‎channeldb/forwarding_package_test.go‎

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,31 @@ func checkPkgFilterEncodeDecode(t *testing.T, i uint16, f *channeldb.PkgFilter)
142142

143143
var (
144144
chanID = lnwire.NewChanIDFromOutPoint(wire.OutPoint{})
145+
)
146+
147+
func testSettleFails() []channeldb.LogUpdate {
148+
return []channeldb.LogUpdate{
149+
{
150+
LogIndex: 2,
151+
UpdateMsg: &lnwire.UpdateFulfillHTLC{
152+
ChanID: chanID,
153+
ID: 0,
154+
PaymentPreimage: [32]byte{0},
155+
},
156+
},
157+
{
158+
LogIndex: 3,
159+
UpdateMsg: &lnwire.UpdateFailHTLC{
160+
ChanID: chanID,
161+
ID: 1,
162+
Reason: []byte{},
163+
},
164+
},
165+
}
166+
}
145167

146-
adds = []channeldb.LogUpdate{
168+
func testAdds() []channeldb.LogUpdate {
169+
return []channeldb.LogUpdate{
147170
{
148171
LogIndex: 0,
149172
UpdateMsg: &lnwire.UpdateAddHTLC{
@@ -165,26 +188,7 @@ var (
165188
},
166189
},
167190
}
168-
169-
settleFails = []channeldb.LogUpdate{
170-
{
171-
LogIndex: 2,
172-
UpdateMsg: &lnwire.UpdateFulfillHTLC{
173-
ChanID: chanID,
174-
ID: 0,
175-
PaymentPreimage: [32]byte{0},
176-
},
177-
},
178-
{
179-
LogIndex: 3,
180-
UpdateMsg: &lnwire.UpdateFailHTLC{
181-
ChanID: chanID,
182-
ID: 1,
183-
Reason: []byte{},
184-
},
185-
},
186-
}
187-
)
191+
}
188192

189193
// TestPackagerEmptyFwdPkg checks that the state transitions exhibited by a
190194
// forwarding package that contains no adds, fails or settles. We expect that
@@ -273,6 +277,8 @@ func TestPackagerOnlyAdds(t *testing.T) {
273277
t.Fatalf("no forwarding packages should exist, found %d", len(fwdPkgs))
274278
}
275279

280+
adds := testAdds()
281+
276282
// Next, create and write a new forwarding package that only has add
277283
// htlcs.
278284
fwdPkg := channeldb.NewFwdPkg(shortChanID, 0, adds, nil)
@@ -377,6 +383,7 @@ func TestPackagerOnlySettleFails(t *testing.T) {
377383

378384
// Next, create and write a new forwarding package that only has add
379385
// htlcs.
386+
settleFails := testSettleFails()
380387
fwdPkg := channeldb.NewFwdPkg(shortChanID, 0, nil, settleFails)
381388

382389
nSettleFails := len(settleFails)
@@ -479,8 +486,11 @@ func TestPackagerAddsThenSettleFails(t *testing.T) {
479486
t.Fatalf("no forwarding packages should exist, found %d", len(fwdPkgs))
480487
}
481488

489+
adds := testAdds()
490+
482491
// Next, create and write a new forwarding package that only has add
483492
// htlcs.
493+
settleFails := testSettleFails()
484494
fwdPkg := channeldb.NewFwdPkg(shortChanID, 0, adds, settleFails)
485495

486496
nAdds := len(adds)
@@ -612,8 +622,11 @@ func TestPackagerSettleFailsThenAdds(t *testing.T) {
612622
t.Fatalf("no forwarding packages should exist, found %d", len(fwdPkgs))
613623
}
614624

625+
adds := testAdds()
626+
615627
// Next, create and write a new forwarding package that has both add
616628
// and settle/fail htlcs.
629+
settleFails := testSettleFails()
617630
fwdPkg := channeldb.NewFwdPkg(shortChanID, 0, adds, settleFails)
618631

619632
nAdds := len(adds)

0 commit comments

Comments
 (0)