Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

core, miner, rpc, eth: fix goroutine leaks in tests #24211

Merged
merged 2 commits into from Jan 21, 2022
Merged

core, miner, rpc, eth: fix goroutine leaks in tests #24211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7855d61
fix blocking and non-blocking issues
charlesxsh Jan 14, 2022
c7add2e
core: revert change in blockchain.go
holiman Jan 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 10 additions & 9 deletions core/blockchain_repair_test.go
View file
Open in desktop
Expand Up @@ -1779,6 +1779,7 @@ func testRepair(t *testing.T, tt *rewindTest, snapshots bool) {
SnapshotLimit: 0, // Disable snapshot by default
}
)
defer engine.Close()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to close the FullFaker ethash engine? There is nothing to do in the Close function

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right it can't be necessary, but doing it is more correct, so I'd rather just keep it htere

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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