From d2c971e842993a676c9a82a5846ed5227f7f5818 Mon Sep 17 00:00:00 2001 From: Shihao Xia Date: Fri, 21 Jan 2022 06:22:44 -0500 Subject: [PATCH] core, miner, rpc, eth: fix goroutine leaks in tests (#24211) * fix blocking and non-blocking issues * core: revert change in blockchain.go Co-authored-by: Martin Holst Swende --- core/blockchain_repair_test.go | 19 ++++++++-------- eth/fetcher/block_fetcher_test.go | 4 +++- graphql/graphql_test.go | 1 + internal/jsre/jsre_test.go | 8 +++---- miner/miner_test.go | 36 ++++++++++++++++++++++--------- miner/worker_test.go | 2 +- node/node_test.go | 2 +- rpc/client_test.go | 1 + signer/core/api_test.go | 3 +++ 9 files changed, 50 insertions(+), 26 deletions(-) diff --git a/core/blockchain_repair_test.go b/core/blockchain_repair_test.go index eb5025ed55e78..9133671796582 100644 --- a/core/blockchain_repair_test.go +++ b/core/blockchain_repair_test.go @@ -1779,6 +1779,7 @@ func testRepair(t *testing.T, tt *rewindTest, snapshots bool) { SnapshotLimit: 0, // Disable snapshot by default } ) + defer engine.Close() if snapshots { config.SnapshotLimit = 256 config.SnapshotWait = true @@ -1836,25 +1837,25 @@ func testRepair(t *testing.T, tt *rewindTest, snapshots bool) { } defer db.Close() - chain, err = NewBlockChain(db, nil, params.AllEthashProtocolChanges, engine, vm.Config{}, nil, nil) + newChain, err := NewBlockChain(db, nil, params.AllEthashProtocolChanges, engine, vm.Config{}, nil, nil) if err != nil { t.Fatalf("Failed to recreate chain: %v", err) } - defer chain.Stop() + defer newChain.Stop() // Iterate over all the remaining blocks and ensure there are no gaps - verifyNoGaps(t, chain, true, canonblocks) - verifyNoGaps(t, chain, false, sideblocks) - verifyCutoff(t, chain, true, canonblocks, tt.expCanonicalBlocks) - verifyCutoff(t, chain, false, sideblocks, tt.expSidechainBlocks) + verifyNoGaps(t, newChain, true, canonblocks) + verifyNoGaps(t, newChain, false, sideblocks) + verifyCutoff(t, newChain, true, canonblocks, tt.expCanonicalBlocks) + verifyCutoff(t, newChain, false, sideblocks, tt.expSidechainBlocks) - if head := chain.CurrentHeader(); head.Number.Uint64() != tt.expHeadHeader { + if head := newChain.CurrentHeader(); head.Number.Uint64() != tt.expHeadHeader { t.Errorf("Head header mismatch: have %d, want %d", head.Number, tt.expHeadHeader) } - if head := chain.CurrentFastBlock(); head.NumberU64() != tt.expHeadFastBlock { + if head := newChain.CurrentFastBlock(); head.NumberU64() != tt.expHeadFastBlock { t.Errorf("Head fast block mismatch: have %d, want %d", head.NumberU64(), tt.expHeadFastBlock) } - if head := chain.CurrentBlock(); head.NumberU64() != tt.expHeadBlock { + if head := newChain.CurrentBlock(); head.NumberU64() != tt.expHeadBlock { t.Errorf("Head block mismatch: have %d, want %d", head.NumberU64(), tt.expHeadBlock) } if frozen, err := db.(freezer).Ancients(); err != nil { diff --git a/eth/fetcher/block_fetcher_test.go b/eth/fetcher/block_fetcher_test.go index 628a5650424d2..06c61ae55d205 100644 --- a/eth/fetcher/block_fetcher_test.go +++ b/eth/fetcher/block_fetcher_test.go @@ -364,6 +364,7 @@ func testSequentialAnnouncements(t *testing.T, light bool) { hashes, blocks := makeChain(targetBlocks, 0, genesis) tester := newTester(light) + defer tester.fetcher.Stop() headerFetcher := tester.makeHeaderFetcher("valid", blocks, -gatherSlack) bodyFetcher := tester.makeBodyFetcher("valid", blocks, 0) @@ -743,7 +744,7 @@ func testInvalidNumberAnnouncement(t *testing.T, light bool) { badBodyFetcher := tester.makeBodyFetcher("bad", blocks, 0) imported := make(chan interface{}) - announced := make(chan interface{}) + announced := make(chan interface{}, 2) tester.fetcher.importedHook = func(header *types.Header, block *types.Block) { if light { if header == nil { @@ -806,6 +807,7 @@ func TestEmptyBlockShortCircuit(t *testing.T) { hashes, blocks := makeChain(32, 0, genesis) tester := newTester(false) + defer tester.fetcher.Stop() headerFetcher := tester.makeHeaderFetcher("valid", blocks, -gatherSlack) bodyFetcher := tester.makeBodyFetcher("valid", blocks, 0) diff --git a/graphql/graphql_test.go b/graphql/graphql_test.go index 4e0f099e42089..a0b7979069278 100644 --- a/graphql/graphql_test.go +++ b/graphql/graphql_test.go @@ -48,6 +48,7 @@ func TestBuildSchema(t *testing.T) { conf := node.DefaultConfig conf.DataDir = ddir stack, err := node.New(&conf) + defer stack.Close() if err != nil { t.Fatalf("could not create new node: %v", err) } diff --git a/internal/jsre/jsre_test.go b/internal/jsre/jsre_test.go index bc38f7a44a86f..57acdaed90ae0 100644 --- a/internal/jsre/jsre_test.go +++ b/internal/jsre/jsre_test.go @@ -83,20 +83,20 @@ func TestNatto(t *testing.T) { err := jsre.Exec("test.js") if err != nil { - t.Errorf("expected no error, got %v", err) + t.Fatalf("expected no error, got %v", err) } time.Sleep(100 * time.Millisecond) val, err := jsre.Run("msg") if err != nil { - t.Errorf("expected no error, got %v", err) + t.Fatalf("expected no error, got %v", err) } if val.ExportType().Kind() != reflect.String { - t.Errorf("expected string value, got %v", val) + t.Fatalf("expected string value, got %v", val) } exp := "testMsg" got := val.ToString().String() if exp != got { - t.Errorf("expected '%v', got '%v'", exp, got) + t.Fatalf("expected '%v', got '%v'", exp, got) } jsre.Stop(false) } diff --git a/miner/miner_test.go b/miner/miner_test.go index de7ca73e260eb..0b8595dac7ea5 100644 --- a/miner/miner_test.go +++ b/miner/miner_test.go @@ -80,7 +80,8 @@ func (bc *testBlockChain) SubscribeChainHeadEvent(ch chan<- core.ChainHeadEvent) } func TestMiner(t *testing.T) { - miner, mux := createMiner(t) + miner, mux, cleanup := createMiner(t) + defer cleanup(false) miner.Start(common.HexToAddress("0x12345")) waitForMiningState(t, miner, true) // Start the downloader @@ -107,7 +108,8 @@ func TestMiner(t *testing.T) { // An initial FailedEvent should allow mining to stop on a subsequent // downloader StartEvent. func TestMinerDownloaderFirstFails(t *testing.T) { - miner, mux := createMiner(t) + miner, mux, cleanup := createMiner(t) + defer cleanup(false) miner.Start(common.HexToAddress("0x12345")) waitForMiningState(t, miner, true) // Start the downloader @@ -138,8 +140,8 @@ func TestMinerDownloaderFirstFails(t *testing.T) { } func TestMinerStartStopAfterDownloaderEvents(t *testing.T) { - miner, mux := createMiner(t) - + miner, mux, cleanup := createMiner(t) + defer cleanup(false) miner.Start(common.HexToAddress("0x12345")) waitForMiningState(t, miner, true) // Start the downloader @@ -161,7 +163,8 @@ func TestMinerStartStopAfterDownloaderEvents(t *testing.T) { } func TestStartWhileDownload(t *testing.T) { - miner, mux := createMiner(t) + miner, mux, cleanup := createMiner(t) + defer cleanup(false) waitForMiningState(t, miner, false) miner.Start(common.HexToAddress("0x12345")) waitForMiningState(t, miner, true) @@ -174,16 +177,19 @@ func TestStartWhileDownload(t *testing.T) { } func TestStartStopMiner(t *testing.T) { - miner, _ := createMiner(t) + miner, _, cleanup := createMiner(t) + defer cleanup(false) waitForMiningState(t, miner, false) miner.Start(common.HexToAddress("0x12345")) waitForMiningState(t, miner, true) miner.Stop() waitForMiningState(t, miner, false) + } func TestCloseMiner(t *testing.T) { - miner, _ := createMiner(t) + miner, _, cleanup := createMiner(t) + defer cleanup(true) waitForMiningState(t, miner, false) miner.Start(common.HexToAddress("0x12345")) waitForMiningState(t, miner, true) @@ -195,7 +201,8 @@ func TestCloseMiner(t *testing.T) { // TestMinerSetEtherbase checks that etherbase becomes set even if mining isn't // possible at the moment func TestMinerSetEtherbase(t *testing.T) { - miner, mux := createMiner(t) + miner, mux, cleanup := createMiner(t) + defer cleanup(false) // Start with a 'bad' mining address miner.Start(common.HexToAddress("0xdead")) waitForMiningState(t, miner, true) @@ -230,7 +237,7 @@ func waitForMiningState(t *testing.T, m *Miner, mining bool) { t.Fatalf("Mining() == %t, want %t", state, mining) } -func createMiner(t *testing.T) (*Miner, *event.TypeMux) { +func createMiner(t *testing.T) (*Miner, *event.TypeMux, func(skipMiner bool)) { // Create Ethash config config := Config{ Etherbase: common.HexToAddress("123456789"), @@ -259,5 +266,14 @@ func createMiner(t *testing.T) (*Miner, *event.TypeMux) { // Create event Mux mux := new(event.TypeMux) // Create Miner - return New(backend, &config, chainConfig, mux, engine, nil, merger), mux + miner := New(backend, &config, chainConfig, mux, engine, nil, merger) + cleanup := func(skipMiner bool) { + bc.Stop() + engine.Close() + pool.Stop() + if !skipMiner { + miner.Close() + } + } + return miner, mux, cleanup } diff --git a/miner/worker_test.go b/miner/worker_test.go index c8ddd2c320b81..bbbff745bf3b9 100644 --- a/miner/worker_test.go +++ b/miner/worker_test.go @@ -382,7 +382,7 @@ func testRegenerateMiningBlock(t *testing.T, chainConfig *params.ChainConfig, en w, b := newTestWorker(t, chainConfig, engine, rawdb.NewMemoryDatabase(), 0) defer w.close() - var taskCh = make(chan struct{}) + var taskCh = make(chan struct{}, 3) taskIndex := 0 w.newTaskHook = func(task *task) { diff --git a/node/node_test.go b/node/node_test.go index e104630600042..25cfa9d38d78f 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -393,7 +393,7 @@ func TestLifecycleTerminationGuarantee(t *testing.T) { // on the given prefix func TestRegisterHandler_Successful(t *testing.T) { node := createNode(t, 7878, 7979) - + defer node.Close() // create and mount handler handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Write([]byte("success")) diff --git a/rpc/client_test.go b/rpc/client_test.go index 224eb0c5c828a..fa6010bb199c1 100644 --- a/rpc/client_test.go +++ b/rpc/client_test.go @@ -615,6 +615,7 @@ func TestClientReconnect(t *testing.T) { // Start a server and corresponding client. s1, l1 := startServer("127.0.0.1:0") client, err := DialContext(ctx, "ws://"+l1.Addr().String()) + defer client.Close() if err != nil { t.Fatal("can't dial", err) } diff --git a/signer/core/api_test.go b/signer/core/api_test.go index 36f12f71a52d1..9f44ca3195664 100644 --- a/signer/core/api_test.go +++ b/signer/core/api_test.go @@ -256,6 +256,9 @@ func TestSignTx(t *testing.T) { if err != nil { t.Fatal(err) } + if len(list) == 0 { + t.Fatal("Unexpected empty list") + } a := common.NewMixedcaseAddress(list[0]) methodSig := "test(uint)"