From 17ce0a37de5a2712a8bf9d58df705e718b3b2cd6 Mon Sep 17 00:00:00 2001
From: Ivan Daniluk <ivan.daniluk@gmail.com>
Date: Tue, 8 Aug 2017 20:10:09 +0300
Subject: [PATCH] eth/downloader: fix race in downloadTesterPeer (#14942)

* eth/downloader: fix race in downloadTesterPeer

Signed-off-by: Ivan Daniluk <ivan.daniluk@gmail.com>

* eth/downloader: minor datarace fix cleanup
---
 eth/downloader/downloader_test.go | 32 ++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/eth/downloader/downloader_test.go b/eth/downloader/downloader_test.go
index 36e8c800f6..d66aafe94e 100644
--- a/eth/downloader/downloader_test.go
+++ b/eth/downloader/downloader_test.go
@@ -403,7 +403,7 @@ func (dl *downloadTester) newSlowPeer(id string, version int, hashes []common.Ha
 	dl.lock.Lock()
 	defer dl.lock.Unlock()
 
-	var err = dl.downloader.RegisterPeer(id, version, &downloadTesterPeer{dl, id, delay})
+	var err = dl.downloader.RegisterPeer(id, version, &downloadTesterPeer{dl: dl, id: id, delay: delay})
 	if err == nil {
 		// Assign the owned hashes, headers and blocks to the peer (deep copy)
 		dl.peerHashes[id] = make([]common.Hash, len(hashes))
@@ -465,6 +465,24 @@ type downloadTesterPeer struct {
 	dl    *downloadTester
 	id    string
 	delay time.Duration
+	lock  sync.RWMutex
+}
+
+// setDelay is a thread safe setter for the network delay value.
+func (dlp *downloadTesterPeer) setDelay(delay time.Duration) {
+	dlp.lock.Lock()
+	defer dlp.lock.Unlock()
+
+	dlp.delay = delay
+}
+
+// waitDelay is a thread safe way to sleep for the configured time.
+func (dlp *downloadTesterPeer) waitDelay() {
+	dlp.lock.RLock()
+	delay := dlp.delay
+	dlp.lock.RUnlock()
+
+	time.Sleep(delay)
 }
 
 // Head constructs a function to retrieve a peer's current head hash
@@ -499,7 +517,7 @@ func (dlp *downloadTesterPeer) RequestHeadersByHash(origin common.Hash, amount i
 // origin; associated with a particular peer in the download tester. The returned
 // function can be used to retrieve batches of headers from the particular peer.
 func (dlp *downloadTesterPeer) RequestHeadersByNumber(origin uint64, amount int, skip int, reverse bool) error {
-	time.Sleep(dlp.delay)
+	dlp.waitDelay()
 
 	dlp.dl.lock.RLock()
 	defer dlp.dl.lock.RUnlock()
@@ -525,7 +543,7 @@ func (dlp *downloadTesterPeer) RequestHeadersByNumber(origin uint64, amount int,
 // peer in the download tester. The returned function can be used to retrieve
 // batches of block bodies from the particularly requested peer.
 func (dlp *downloadTesterPeer) RequestBodies(hashes []common.Hash) error {
-	time.Sleep(dlp.delay)
+	dlp.waitDelay()
 
 	dlp.dl.lock.RLock()
 	defer dlp.dl.lock.RUnlock()
@@ -550,7 +568,7 @@ func (dlp *downloadTesterPeer) RequestBodies(hashes []common.Hash) error {
 // peer in the download tester. The returned function can be used to retrieve
 // batches of block receipts from the particularly requested peer.
 func (dlp *downloadTesterPeer) RequestReceipts(hashes []common.Hash) error {
-	time.Sleep(dlp.delay)
+	dlp.waitDelay()
 
 	dlp.dl.lock.RLock()
 	defer dlp.dl.lock.RUnlock()
@@ -572,7 +590,7 @@ func (dlp *downloadTesterPeer) RequestReceipts(hashes []common.Hash) error {
 // peer in the download tester. The returned function can be used to retrieve
 // batches of node state data from the particularly requested peer.
 func (dlp *downloadTesterPeer) RequestNodeData(hashes []common.Hash) error {
-	time.Sleep(dlp.delay)
+	dlp.waitDelay()
 
 	dlp.dl.lock.RLock()
 	defer dlp.dl.lock.RUnlock()
@@ -1746,7 +1764,7 @@ func testFastCriticalRestarts(t *testing.T, protocol int, progress bool) {
 	for i := 0; i < fsPivotInterval; i++ {
 		tester.peerMissingStates["peer"][headers[hashes[fsMinFullBlocks+i]].Root] = true
 	}
-	(tester.downloader.peers.peers["peer"].peer).(*downloadTesterPeer).delay = 500 * time.Millisecond // Enough to reach the critical section
+	(tester.downloader.peers.peers["peer"].peer).(*downloadTesterPeer).setDelay(500 * time.Millisecond) // Enough to reach the critical section
 
 	// Synchronise with the peer a few times and make sure they fail until the retry limit
 	for i := 0; i < int(fsCriticalTrials)-1; i++ {
@@ -1765,7 +1783,7 @@ func testFastCriticalRestarts(t *testing.T, protocol int, progress bool) {
 			tester.lock.Lock()
 			tester.peerHeaders["peer"][hashes[fsMinFullBlocks-1]] = headers[hashes[fsMinFullBlocks-1]]
 			tester.peerMissingStates["peer"] = map[common.Hash]bool{tester.downloader.fsPivotLock.Root: true}
-			(tester.downloader.peers.peers["peer"].peer).(*downloadTesterPeer).delay = 0
+			(tester.downloader.peers.peers["peer"].peer).(*downloadTesterPeer).setDelay(0)
 			tester.lock.Unlock()
 		}
 	}