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.

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

Eliminate deadlock by using generate instead of setgenerate RPC #88

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions comm.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func (com *Communication) Start(actors []*Actor, node *Node, txCurve map[int32]*
}
}

// Start mining.
// Start a miner process.
miner, err := NewMiner(miningAddrs, com.exit, com.height, com.txpool)
if err != nil {
close(com.exit)
Expand Down Expand Up @@ -428,6 +428,12 @@ func (com *Communication) estimateTpb(tpbChan chan<- int) {
func (com *Communication) Communicate(txCurve map[int32]*Row, miner *Miner, actors []*Actor) {
defer com.wg.Done()

go func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can do without the goroutine since we're waiting for the blocks in the for select below anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I see that generate is blocking, unlike setgenerate so actually we could do away with the notifications handlers to wait for the miner. Let me check that.

if err := miner.Generate(uint32(*startBlock) - 1); err != nil {
close(com.exit)
}
}()

for {
select {
case h := <-com.height:
Expand All @@ -438,12 +444,6 @@ func (com *Communication) Communicate(txCurve map[int32]*Row, miner *Miner, acto
return
}

// disable mining until the required no. of tx are in mempool
if err := miner.StopMining(); err != nil {
close(com.exit)
return
}

// wait until this block is processed
select {
case <-com.blockQueue.processed:
Expand Down Expand Up @@ -575,10 +575,11 @@ func (com *Communication) Communicate(txCurve map[int32]*Row, miner *Miner, acto
log.Printf("Waiting for miner...")
wg.Wait()
// mine the above tx in the next block
if err := miner.StartMining(); err != nil {
close(com.exit)
return
}
go func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

if err := miner.Generate(1); err != nil {
close(com.exit)
}
}()
case <-com.exit:
return
}
Expand Down
14 changes: 9 additions & 5 deletions miner.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,6 @@ func NewMiner(miningAddrs []btcutil.Address, exit chan struct{},
return miner, err
}

// Use just one core for mining.
if err := miner.StartMining(); err != nil {
return miner, err
}

// Register for block notifications.
if err := miner.client.NotifyBlocks(); err != nil {
log.Printf("%s: Cannot register for block notifications: %v", miner, err)
Expand Down Expand Up @@ -139,3 +134,12 @@ func (m *Miner) StopMining() error {
}
return nil
}

// Generate makes the CPU miner mine the requested number of blocks
Copy link
Contributor

Choose a reason for hiding this comment

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

This would make StartMining and StopMining obsolete, so we can remove those.

func (m *Miner) Generate(numBlocks uint32) error {
if _, err := m.client.Generate(numBlocks); err != nil {
log.Printf("%s: Cannot generate %d blocks: %v", m, numBlocks, err)
return err
}
return nil
}