Add patch to fix race condition in `Downloader.Cancel`

- Submitted patch upstream in https://github.com/ethereum/go-ethereum/pull/16585
- Fixes Jenkins build in https://jenkins.status.im/job/status-go/job/race-check/48/
This commit is contained in:
Pedro Pombeiro 2018-04-27 11:09:55 +02:00 committed by Pedro Pombeiro
parent 94da262a0b
commit 7074048fa5
3 changed files with 43 additions and 20 deletions

View File

@ -1,5 +1,5 @@
diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go
index 7ede530a9..60ff8dbc6 100644 index cbdaa0ca2..60ff8dbc6 100644
--- a/eth/downloader/downloader.go --- a/eth/downloader/downloader.go
+++ b/eth/downloader/downloader.go +++ b/eth/downloader/downloader.go
@@ -143,6 +143,8 @@ @@ -143,6 +143,8 @@
@ -11,15 +11,7 @@ index 7ede530a9..60ff8dbc6 100644
// Testing hooks // Testing hooks
syncInitHook func(uint64, uint64) // Method to call upon initiating a new sync run syncInitHook func(uint64, uint64) // Method to call upon initiating a new sync run
bodyFetchHook func([]*types.Header) // Method to call upon starting a block body fetch bodyFetchHook func([]*types.Header) // Method to call upon starting a block body fetch
@@ -228,6 +230,7 @@ @@ -403,7 +405,9 @@
},
trackStateReq: make(chan *stateReq),
}
+ dl.downloads.Add(1) // Add a sentinel value to avoid waiting with zero count and getting a race condition signaled when terminating Downloader before starting sync
go dl.qosTuner()
go dl.stateFetcher()
return dl
@@ -403,7 +406,9 @@
// specified peer and head hash. // specified peer and head hash.
func (d *Downloader) syncWithPeer(p *peerConnection, hash common.Hash, td *big.Int) (err error) { func (d *Downloader) syncWithPeer(p *peerConnection, hash common.Hash, td *big.Int) (err error) {
d.mux.Post(StartEvent{}) d.mux.Post(StartEvent{})
@ -29,14 +21,31 @@ index 7ede530a9..60ff8dbc6 100644
// reset on error // reset on error
if err != nil { if err != nil {
d.mux.Post(FailedEvent{err}) d.mux.Post(FailedEvent{err})
@@ -539,6 +544,13 @@ @@ -471,12 +475,17 @@
} else if d.mode == FullSync {
fetchers = append(fetchers, d.processFullSyncContent)
}
- return d.spawnSync(fetchers)
+ return d.spawnSync(errCancelHeaderFetch, fetchers)
}
// 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 {
+func (d *Downloader) spawnSync(errCancel error, fetchers []func() error) error {
+ select {
+ case <-d.cancelCh:
+ return errCancel
+ default:
+ }
errc := make(chan error, len(fetchers))
d.cancelWg.Add(len(fetchers))
for _, fn := range fetchers {
@@ -539,6 +548,10 @@
// Cancel any pending download requests // Cancel any pending download requests
d.Cancel() d.Cancel()
+ +
+ // Remove sentinel value from WaitGroup before calling Wait()
+ go func() { d.downloads.Done() }()
+
+ // Wait, so external dependencies aren't destroyed + // Wait, so external dependencies aren't destroyed
+ // until the download processing is done. + // until the download processing is done.
+ d.downloads.Wait() + d.downloads.Wait()

View File

@ -0,0 +1,13 @@
diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go
index eb64f05a..3e8c9cb1 100644
--- a/eth/downloader/downloader.go
+++ b/eth/downloader/downloader.go
@@ -230,6 +230,7 @@ func New(mode SyncMode, stateDb ethdb.Database, mux *event.TypeMux, chain BlockC
},
trackStateReq: make(chan *stateReq),
}
+ dl.cancelWg.Add(1) // Add a sentinel to avoid getting flagged as a race condition in case Cancel is called before spawnSync
dl.downloads.Add(1) // Add a sentinel value to avoid waiting with zero count and getting a race condition signaled when terminating Downloader before starting sync
go dl.qosTuner()
go dl.stateFetcher()

View File

@ -230,7 +230,6 @@ func New(mode SyncMode, stateDb ethdb.Database, mux *event.TypeMux, chain BlockC
}, },
trackStateReq: make(chan *stateReq), trackStateReq: make(chan *stateReq),
} }
dl.downloads.Add(1) // Add a sentinel value to avoid waiting with zero count and getting a race condition signaled when terminating Downloader before starting sync
go dl.qosTuner() go dl.qosTuner()
go dl.stateFetcher() go dl.stateFetcher()
return dl return dl
@ -476,12 +475,17 @@ func (d *Downloader) syncWithPeer(p *peerConnection, hash common.Hash, td *big.I
} else if d.mode == FullSync { } else if d.mode == FullSync {
fetchers = append(fetchers, d.processFullSyncContent) fetchers = append(fetchers, d.processFullSyncContent)
} }
return d.spawnSync(fetchers) return d.spawnSync(errCancelHeaderFetch, fetchers)
} }
// spawnSync runs d.process and all given fetcher functions to completion in // spawnSync runs d.process and all given fetcher functions to completion in
// separate goroutines, returning the first error that appears. // separate goroutines, returning the first error that appears.
func (d *Downloader) spawnSync(fetchers []func() error) error { func (d *Downloader) spawnSync(errCancel error, fetchers []func() error) error {
select {
case <-d.cancelCh:
return errCancel
default:
}
errc := make(chan error, len(fetchers)) errc := make(chan error, len(fetchers))
d.cancelWg.Add(len(fetchers)) d.cancelWg.Add(len(fetchers))
for _, fn := range fetchers { for _, fn := range fetchers {
@ -545,9 +549,6 @@ func (d *Downloader) Terminate() {
// Cancel any pending download requests // Cancel any pending download requests
d.Cancel() d.Cancel()
// Remove sentinel value from WaitGroup before calling Wait()
go func() { d.downloads.Done() }()
// Wait, so external dependencies aren't destroyed // Wait, so external dependencies aren't destroyed
// until the download processing is done. // until the download processing is done.
d.downloads.Wait() d.downloads.Wait()