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
This commit is contained in:
		
				
					committed by
					
						
						Péter Szilágyi
					
				
			
			
				
	
			
			
			
						parent
						
							26b2d6e1aa
						
					
				
				
					commit
					17ce0a37de
				
			@@ -403,7 +403,7 @@ func (dl *downloadTester) newSlowPeer(id string, version int, hashes []common.Ha
 | 
				
			|||||||
	dl.lock.Lock()
 | 
						dl.lock.Lock()
 | 
				
			||||||
	defer dl.lock.Unlock()
 | 
						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 {
 | 
						if err == nil {
 | 
				
			||||||
		// Assign the owned hashes, headers and blocks to the peer (deep copy)
 | 
							// Assign the owned hashes, headers and blocks to the peer (deep copy)
 | 
				
			||||||
		dl.peerHashes[id] = make([]common.Hash, len(hashes))
 | 
							dl.peerHashes[id] = make([]common.Hash, len(hashes))
 | 
				
			||||||
@@ -465,6 +465,24 @@ type downloadTesterPeer struct {
 | 
				
			|||||||
	dl    *downloadTester
 | 
						dl    *downloadTester
 | 
				
			||||||
	id    string
 | 
						id    string
 | 
				
			||||||
	delay time.Duration
 | 
						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
 | 
					// 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
 | 
					// 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.
 | 
					// 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 {
 | 
					func (dlp *downloadTesterPeer) RequestHeadersByNumber(origin uint64, amount int, skip int, reverse bool) error {
 | 
				
			||||||
	time.Sleep(dlp.delay)
 | 
						dlp.waitDelay()
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	dlp.dl.lock.RLock()
 | 
						dlp.dl.lock.RLock()
 | 
				
			||||||
	defer dlp.dl.lock.RUnlock()
 | 
						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
 | 
					// peer in the download tester. The returned function can be used to retrieve
 | 
				
			||||||
// batches of block bodies from the particularly requested peer.
 | 
					// batches of block bodies from the particularly requested peer.
 | 
				
			||||||
func (dlp *downloadTesterPeer) RequestBodies(hashes []common.Hash) error {
 | 
					func (dlp *downloadTesterPeer) RequestBodies(hashes []common.Hash) error {
 | 
				
			||||||
	time.Sleep(dlp.delay)
 | 
						dlp.waitDelay()
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	dlp.dl.lock.RLock()
 | 
						dlp.dl.lock.RLock()
 | 
				
			||||||
	defer dlp.dl.lock.RUnlock()
 | 
						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
 | 
					// peer in the download tester. The returned function can be used to retrieve
 | 
				
			||||||
// batches of block receipts from the particularly requested peer.
 | 
					// batches of block receipts from the particularly requested peer.
 | 
				
			||||||
func (dlp *downloadTesterPeer) RequestReceipts(hashes []common.Hash) error {
 | 
					func (dlp *downloadTesterPeer) RequestReceipts(hashes []common.Hash) error {
 | 
				
			||||||
	time.Sleep(dlp.delay)
 | 
						dlp.waitDelay()
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	dlp.dl.lock.RLock()
 | 
						dlp.dl.lock.RLock()
 | 
				
			||||||
	defer dlp.dl.lock.RUnlock()
 | 
						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
 | 
					// peer in the download tester. The returned function can be used to retrieve
 | 
				
			||||||
// batches of node state data from the particularly requested peer.
 | 
					// batches of node state data from the particularly requested peer.
 | 
				
			||||||
func (dlp *downloadTesterPeer) RequestNodeData(hashes []common.Hash) error {
 | 
					func (dlp *downloadTesterPeer) RequestNodeData(hashes []common.Hash) error {
 | 
				
			||||||
	time.Sleep(dlp.delay)
 | 
						dlp.waitDelay()
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	dlp.dl.lock.RLock()
 | 
						dlp.dl.lock.RLock()
 | 
				
			||||||
	defer dlp.dl.lock.RUnlock()
 | 
						defer dlp.dl.lock.RUnlock()
 | 
				
			||||||
@@ -1746,7 +1764,7 @@ func testFastCriticalRestarts(t *testing.T, protocol int, progress bool) {
 | 
				
			|||||||
	for i := 0; i < fsPivotInterval; i++ {
 | 
						for i := 0; i < fsPivotInterval; i++ {
 | 
				
			||||||
		tester.peerMissingStates["peer"][headers[hashes[fsMinFullBlocks+i]].Root] = true
 | 
							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
 | 
						// Synchronise with the peer a few times and make sure they fail until the retry limit
 | 
				
			||||||
	for i := 0; i < int(fsCriticalTrials)-1; i++ {
 | 
						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.lock.Lock()
 | 
				
			||||||
			tester.peerHeaders["peer"][hashes[fsMinFullBlocks-1]] = headers[hashes[fsMinFullBlocks-1]]
 | 
								tester.peerHeaders["peer"][hashes[fsMinFullBlocks-1]] = headers[hashes[fsMinFullBlocks-1]]
 | 
				
			||||||
			tester.peerMissingStates["peer"] = map[common.Hash]bool{tester.downloader.fsPivotLock.Root: true}
 | 
								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()
 | 
								tester.lock.Unlock()
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user