Skip to content

Commit

Permalink
core, miner, rpc, eth: fix goroutine leaks in tests (ethereum#24211)
Browse files Browse the repository at this point in the history
* fix blocking and non-blocking issues

* core: revert change in blockchain.go

Co-authored-by: Martin Holst Swende <martin@swende.se>
  • Loading branch information
2 people authored and jagdeep sidhu committed Jan 21, 2022
1 parent 4255d80 commit 36733a9
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 26 deletions.
19 changes: 10 additions & 9 deletions core/blockchain_repair_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion eth/fetcher/block_fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)

Expand Down
1 change: 1 addition & 0 deletions graphql/graphql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
8 changes: 4 additions & 4 deletions internal/jsre/jsre_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
36 changes: 26 additions & 10 deletions miner/miner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion miner/worker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion node/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
1 change: 1 addition & 0 deletions rpc/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
3 changes: 3 additions & 0 deletions signer/core/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down

0 comments on commit 36733a9

Please sign in to comment.