From e170b4361504c1206bab0a8bce98e788eb06d2c2 Mon Sep 17 00:00:00 2001 From: Andrey Samokhvalov Date: Sun, 9 Jul 2017 01:52:51 +0300 Subject: [PATCH] htlcswitch.test: add server error channel to concurrent access panic This commit where added as a measure to avoid the panic during several server simultanoius fault. The panic happened becuase *t.Testing structure is not concurrent safe. --- htlcswitch/link_test.go | 30 +++++++++++++++++++++++++++++- htlcswitch/mock.go | 22 +++++++++------------- htlcswitch/switch_test.go | 18 +++++++++++------- htlcswitch/test_utils.go | 9 +++++---- 4 files changed, 54 insertions(+), 25 deletions(-) diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 01746bc4..c26f6845 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -152,9 +152,11 @@ func createInterceptorFunc(peer string, messages []expectedMessage, func TestChannelLinkSingleHopPayment(t *testing.T) { t.Parallel() + serverErr := make(chan error, 4) n := newThreeHopNetwork(t, btcutil.SatoshiPerBitcoin*3, btcutil.SatoshiPerBitcoin*5, + serverErr, testStartingHeight, ) if err := n.start(); err != nil { @@ -219,9 +221,11 @@ func TestChannelLinkSingleHopPayment(t *testing.T) { func TestChannelLinkBidirectionalOneHopPayments(t *testing.T) { t.Parallel() + serverErr := make(chan error, 4) n := newThreeHopNetwork(t, btcutil.SatoshiPerBitcoin*3, btcutil.SatoshiPerBitcoin*5, + serverErr, testStartingHeight, ) if err := n.start(); err != nil { @@ -341,9 +345,11 @@ func TestChannelLinkBidirectionalOneHopPayments(t *testing.T) { func TestChannelLinkMultiHopPayment(t *testing.T) { t.Parallel() + serverErr := make(chan error, 4) n := newThreeHopNetwork(t, btcutil.SatoshiPerBitcoin*3, btcutil.SatoshiPerBitcoin*5, + serverErr, testStartingHeight, ) if err := n.start(); err != nil { @@ -438,9 +444,11 @@ func TestChannelLinkMultiHopPayment(t *testing.T) { func TestExitNodeTimelockPayloadMismatch(t *testing.T) { t.Parallel() + serverErr := make(chan error, 4) n := newThreeHopNetwork(t, btcutil.SatoshiPerBitcoin*5, btcutil.SatoshiPerBitcoin*5, + serverErr, testStartingHeight, ) if err := n.start(); err != nil { @@ -484,9 +492,11 @@ func TestExitNodeTimelockPayloadMismatch(t *testing.T) { func TestExitNodeAmountPayloadMismatch(t *testing.T) { t.Parallel() + serverErr := make(chan error, 4) n := newThreeHopNetwork(t, btcutil.SatoshiPerBitcoin*5, btcutil.SatoshiPerBitcoin*5, + serverErr, testStartingHeight, ) if err := n.start(); err != nil { @@ -522,9 +532,11 @@ func TestExitNodeAmountPayloadMismatch(t *testing.T) { func TestLinkForwardTimelockPolicyMismatch(t *testing.T) { t.Parallel() + serverErr := make(chan error, 4) n := newThreeHopNetwork(t, btcutil.SatoshiPerBitcoin*5, btcutil.SatoshiPerBitcoin*5, + serverErr, testStartingHeight, ) if err := n.start(); err != nil { @@ -572,9 +584,11 @@ func TestLinkForwardTimelockPolicyMismatch(t *testing.T) { func TestLinkForwardFeePolicyMismatch(t *testing.T) { t.Parallel() + serverErr := make(chan error, 4) n := newThreeHopNetwork(t, btcutil.SatoshiPerBitcoin*5, btcutil.SatoshiPerBitcoin*5, + serverErr, testStartingHeight, ) if err := n.start(); err != nil { @@ -623,9 +637,11 @@ func TestLinkForwardFeePolicyMismatch(t *testing.T) { func TestLinkForwardMinHTLCPolicyMismatch(t *testing.T) { t.Parallel() + serverErr := make(chan error, 4) n := newThreeHopNetwork(t, btcutil.SatoshiPerBitcoin*5, btcutil.SatoshiPerBitcoin*5, + serverErr, testStartingHeight, ) if err := n.start(); err != nil { @@ -675,9 +691,11 @@ func TestLinkForwardMinHTLCPolicyMismatch(t *testing.T) { func TestUpdateForwardingPolicy(t *testing.T) { t.Parallel() + serverErr := make(chan error, 4) n := newThreeHopNetwork(t, btcutil.SatoshiPerBitcoin*5, btcutil.SatoshiPerBitcoin*5, + serverErr, testStartingHeight, ) if err := n.start(); err != nil { @@ -768,9 +786,11 @@ func TestUpdateForwardingPolicy(t *testing.T) { func TestChannelLinkMultiHopInsufficientPayment(t *testing.T) { t.Parallel() + serverErr := make(chan error, 4) n := newThreeHopNetwork(t, btcutil.SatoshiPerBitcoin*3, btcutil.SatoshiPerBitcoin*5, + serverErr, testStartingHeight, ) if err := n.start(); err != nil { @@ -838,9 +858,11 @@ func TestChannelLinkMultiHopInsufficientPayment(t *testing.T) { func TestChannelLinkMultiHopUnknownPaymentHash(t *testing.T) { t.Parallel() + serverErr := make(chan error, 4) n := newThreeHopNetwork(t, btcutil.SatoshiPerBitcoin*3, btcutil.SatoshiPerBitcoin*5, + serverErr, testStartingHeight, ) if err := n.start(); err != nil { @@ -922,9 +944,11 @@ func TestChannelLinkMultiHopUnknownPaymentHash(t *testing.T) { func TestChannelLinkMultiHopUnknownNextHop(t *testing.T) { t.Parallel() + serverErr := make(chan error, 4) n := newThreeHopNetwork(t, btcutil.SatoshiPerBitcoin*3, btcutil.SatoshiPerBitcoin*5, + serverErr, testStartingHeight, ) if err := n.start(); err != nil { @@ -941,7 +965,7 @@ func TestChannelLinkMultiHopUnknownNextHop(t *testing.T) { htlcAmt, totalTimelock, hops := generateHops(amount, testStartingHeight, n.firstBobChannelLink, n.carolChannelLink) - davePub := newMockServer(t, "save").PubKey() + davePub := newMockServer("save", serverErr).PubKey() invoice, err := n.makePayment(n.aliceServer, n.bobServer, davePub, hops, amount, htlcAmt, totalTimelock) if err == nil { @@ -987,9 +1011,11 @@ func TestChannelLinkMultiHopUnknownNextHop(t *testing.T) { func TestChannelLinkMultiHopDecodeError(t *testing.T) { t.Parallel() + serverErr := make(chan error, 4) n := newThreeHopNetwork(t, btcutil.SatoshiPerBitcoin*3, btcutil.SatoshiPerBitcoin*5, + serverErr, testStartingHeight, ) if err := n.start(); err != nil { @@ -1166,9 +1192,11 @@ func TestChannelLinkExpiryTooSoonMidNode(t *testing.T) { func TestChannelLinkSingleHopMessageOrdering(t *testing.T) { t.Parallel() + serverErr := make(chan error, 4) n := newThreeHopNetwork(t, btcutil.SatoshiPerBitcoin*3, btcutil.SatoshiPerBitcoin*5, + serverErr, testStartingHeight, ) diff --git a/htlcswitch/mock.go b/htlcswitch/mock.go index 2850a142..e7b06202 100644 --- a/htlcswitch/mock.go +++ b/htlcswitch/mock.go @@ -11,8 +11,6 @@ import ( "bytes" - "testing" - "github.com/btcsuite/fastsha256" "github.com/go-errors/errors" "github.com/lightningnetwork/lnd/chainntnfs" @@ -26,17 +24,16 @@ import ( ) type mockServer struct { - sync.Mutex - started int32 shutdown int32 wg sync.WaitGroup - quit chan bool + quit chan struct{} - t *testing.T name string messages chan lnwire.Message + errChan chan error + id [33]byte htlcSwitch *Switch @@ -46,17 +43,17 @@ type mockServer struct { var _ Peer = (*mockServer)(nil) -func newMockServer(t *testing.T, name string) *mockServer { +func newMockServer(name string, errChan chan error) *mockServer { var id [33]byte h := sha256.Sum256([]byte(name)) copy(id[:], h[:]) return &mockServer{ - t: t, + errChan: errChan, id: id, name: name, messages: make(chan lnwire.Message, 3000), - quit: make(chan bool), + quit: make(chan struct{}), registry: newMockRegistry(), htlcSwitch: New(Config{}), interceptorFuncs: make([]messageInterceptor, 0), @@ -94,9 +91,7 @@ func (s *mockServer) Start() error { } if err := s.readHandler(msg); err != nil { - s.Lock() - defer s.Unlock() - s.t.Fatalf("%v server error: %v", s.name, err) + s.errChan <- errors.Errorf("%v server error: %v", s.name, err) } case <-s.quit: return @@ -329,7 +324,8 @@ func (s *mockServer) Disconnect(reason error) { fmt.Printf("server %v disconnected due to %v\n", s.name, reason) s.Stop() - s.t.Fatalf("server %v was disconnected", s.name) + s.errChan <- errors.Errorf("server %v was disconnected: %v", s.name, + reason) } func (s *mockServer) WipeChannel(*lnwallet.LightningChannel) error { diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 11b39942..02d2f417 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -34,8 +34,9 @@ func TestSwitchForward(t *testing.T) { var packet *htlcPacket - alicePeer := newMockServer(t, "alice") - bobPeer := newMockServer(t, "bob") + serverErr := make(chan error, 4) + alicePeer := newMockServer("alice", serverErr) + bobPeer := newMockServer("bob", serverErr) aliceChannelLink := newMockChannelLink(chanID1, aliceChanID, alicePeer) bobChannelLink := newMockChannelLink(chanID2, bobChanID, bobPeer) @@ -112,8 +113,9 @@ func TestSwitchCancel(t *testing.T) { var request *htlcPacket - alicePeer := newMockServer(t, "alice") - bobPeer := newMockServer(t, "bob") + serverErr := make(chan error, 4) + alicePeer := newMockServer("alice", serverErr) + bobPeer := newMockServer("bob", serverErr) aliceChannelLink := newMockChannelLink(chanID1, aliceChanID, alicePeer) bobChannelLink := newMockChannelLink(chanID2, bobChanID, bobPeer) @@ -188,8 +190,9 @@ func TestSwitchAddSamePayment(t *testing.T) { var request *htlcPacket - alicePeer := newMockServer(t, "alice") - bobPeer := newMockServer(t, "bob") + serverErr := make(chan error, 4) + alicePeer := newMockServer("alice", serverErr) + bobPeer := newMockServer("bob", serverErr) aliceChannelLink := newMockChannelLink(chanID1, aliceChanID, alicePeer) bobChannelLink := newMockChannelLink(chanID2, bobChanID, bobPeer) @@ -287,7 +290,8 @@ func TestSwitchAddSamePayment(t *testing.T) { func TestSwitchSendPayment(t *testing.T) { t.Parallel() - alicePeer := newMockServer(t, "alice") + serverErr := make(chan error, 4) + alicePeer := newMockServer("alice", serverErr) aliceChannelLink := newMockChannelLink(chanID1, aliceChanID, alicePeer) s := New(Config{}) diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index 35bd3fa7..6a65875e 100644 --- a/htlcswitch/test_utils.go +++ b/htlcswitch/test_utils.go @@ -508,13 +508,14 @@ func (n *threeHopNetwork) stop() { // channel link channel link channel link channel link // func newThreeHopNetwork(t *testing.T, aliceToBob, - bobToCarol btcutil.Amount, startingHeight uint32) *threeHopNetwork { + bobToCarol btcutil.Amount, serverErr chan error, + startingHeight uint32) *threeHopNetwork { var err error // Create three peers/servers. - aliceServer := newMockServer(t, "alice") - bobServer := newMockServer(t, "bob") - carolServer := newMockServer(t, "carol") + aliceServer := newMockServer("alice", serverErr) + bobServer := newMockServer("bob", serverErr) + carolServer := newMockServer("carol", serverErr) // Create mock decoder instead of sphinx one in order to mock the // route which htlc should follow.