diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go index 9276392e7814..e14775d06a19 100644 --- a/eth/downloader/downloader.go +++ b/eth/downloader/downloader.go @@ -140,9 +140,10 @@ type Downloader struct { stateCh chan dataPack // [eth/63] Channel receiving inbound node state data // Cancellation and termination - cancelPeer string // Identifier of the peer currently being used as the master (cancel on drop) - cancelCh chan struct{} // Channel to cancel mid-flight syncs - cancelLock sync.RWMutex // Lock to protect the cancel channel and peer in delivers + cancelPeer string // Identifier of the peer currently being used as the master (cancel on drop) + cancelCh chan struct{} // Channel to cancel mid-flight syncs + cancelLock sync.RWMutex // Lock to protect the cancel channel and peer in delivers + cancelWg sync.WaitGroup // Make sure all fetcher goroutines have exited. quitCh chan struct{} // Quit channel to signal termination quitLock sync.Mutex // Lock to prevent double closes @@ -312,7 +313,7 @@ func (d *Downloader) UnregisterPeer(id string) error { d.cancelLock.RUnlock() if master { - d.Cancel() + d.cancel() } return nil } @@ -483,12 +484,11 @@ func (d *Downloader) syncWithPeer(p *peerConnection, hash common.Hash, td *big.I // spawnSync runs d.process and all given fetcher functions to completion in // separate goroutines, returning the first error that appears. func (d *Downloader) spawnSync(fetchers []func() error) error { - var wg sync.WaitGroup errc := make(chan error, len(fetchers)) - wg.Add(len(fetchers)) + d.cancelWg.Add(len(fetchers)) for _, fn := range fetchers { fn := fn - go func() { defer wg.Done(); errc <- fn() }() + go func() { defer d.cancelWg.Done(); errc <- fn() }() } // Wait for the first error, then terminate the others. var err error @@ -505,16 +505,16 @@ func (d *Downloader) spawnSync(fetchers []func() error) error { } d.queue.Close() d.Cancel() - wg.Wait() if err == errEnoughBlock { return nil } return err } -// Cancel cancels all of the operations and resets the queue. It returns true -// if the cancel operation was completed. -func (d *Downloader) Cancel() { +// cancel aborts all of the operations and resets the queue. However, cancel does +// not wait for the running download goroutines to finish. This method should be +// used when cancelling the downloads from inside the downloader. +func (d *Downloader) cancel() { // Close the current cancel channel d.cancelLock.Lock() if d.cancelCh != nil { @@ -528,6 +528,13 @@ func (d *Downloader) Cancel() { d.cancelLock.Unlock() } +// Cancel aborts all of the operations and waits for all download goroutines to +// finish before returning. +func (d *Downloader) Cancel() { + d.cancel() + d.cancelWg.Wait() +} + // Terminate interrupts the downloader, canceling all pending operations. // The downloader cannot be reused after calling Terminate. func (d *Downloader) Terminate() {