From 5e3b239ebccdabe6c6a270362cc246fbd64ef347 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 22 May 2018 16:46:55 -0700 Subject: [PATCH 1/2] htlcswitch: ensure we don't attempt to create fwding events for fails In this commit, we fix an existing source of a panic, that could at times lead to a deadlock. If the circuit returned from closeCircuit didn't have an outgoing key (as it was an incomplete forward), then we would attempt to de-ref a nil pointer. This would trigger a panic, and the runtime would start to unwind the stack, and execute each defer in line. A deadlock can arise here, as in the defer at the root goroutine, we need to grab the fwdingEventMtx. However, we already have it at the panic site. We fix this issue by ensuring we only attempt to add the event if it's a _settle_ and also actually has an outgoing circuit (which it should already, just a defensive check). --- htlcswitch/switch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index 94c424a2..52a24035 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -1061,7 +1061,7 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { fail.Reason, ) } - } else { + } else if !isFail && circuit.Outgoing != nil { // If this is an HTLC settle, and it wasn't from a // locally initiated HTLC, then we'll log a forwarding // event so we can flush it to disk later. From b60575fdac731270a7de2e9951af2efc37767333 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 22 May 2018 16:55:08 -0700 Subject: [PATCH 2/2] htlcswitch: run decay log tests in parallel --- htlcswitch/decayedlog_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/htlcswitch/decayedlog_test.go b/htlcswitch/decayedlog_test.go index 6ce217d1..8a702faa 100644 --- a/htlcswitch/decayedlog_test.go +++ b/htlcswitch/decayedlog_test.go @@ -63,6 +63,8 @@ func shutdown(dir string, d sphinx.ReplayLog) { // to delete expired cltv values every time a block is received. Expired cltv // values are cltv values that are < current block height. func TestDecayedLogGarbageCollector(t *testing.T) { + t.Parallel() + d, notifier, hashedSecret, err := startup(true) if err != nil { t.Fatalf("Unable to start up DecayedLog: %v", err) @@ -120,6 +122,8 @@ func TestDecayedLogGarbageCollector(t *testing.T) { // We test that this causes the pair to be deleted even // on GC restarts. func TestDecayedLogPersistentGarbageCollector(t *testing.T) { + t.Parallel() + d, _, hashedSecret, err := startup(true) if err != nil { t.Fatalf("Unable to start up DecayedLog: %v", err) @@ -166,6 +170,8 @@ func TestDecayedLogPersistentGarbageCollector(t *testing.T) { // sharedHashBucket and then deletes it and finally asserts that we can no // longer retrieve it. func TestDecayedLogInsertionAndDeletion(t *testing.T) { + t.Parallel() + d, _, hashedSecret, err := startup(false) if err != nil { t.Fatalf("Unable to start up DecayedLog: %v", err) @@ -200,6 +206,8 @@ func TestDecayedLogInsertionAndDeletion(t *testing.T) { // cltv value is indeed still stored in the sharedHashBucket. We then delete // the cltv value and check that it persists upon startup. func TestDecayedLogStartAndStop(t *testing.T) { + t.Parallel() + d, _, hashedSecret, err := startup(false) if err != nil { t.Fatalf("Unable to start up DecayedLog: %v", err) @@ -262,6 +270,8 @@ func TestDecayedLogStartAndStop(t *testing.T) { // via the nested sharedHashBucket and finally asserts that the original stored // and retrieved cltv values are equal. func TestDecayedLogStorageAndRetrieval(t *testing.T) { + t.Parallel() + d, _, hashedSecret, err := startup(false) if err != nil { t.Fatalf("Unable to start up DecayedLog: %v", err)