From cf7c44a7f6e5422148e0be98186d5570ce3e0ac5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Mon, 15 Jun 2015 15:18:04 +0300 Subject: eth/downloader: detailed comment for the race corner case --- eth/downloader/downloader.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) (limited to 'eth/downloader/downloader.go') diff --git a/eth/downloader/downloader.go b/eth/downloader/downloader.go index 7f8ef12ee..306c4fd2d 100644 --- a/eth/downloader/downloader.go +++ b/eth/downloader/downloader.go @@ -522,9 +522,7 @@ out: peer.Promote() peer.SetIdle() glog.V(logger.Detail).Infof("%s: delivered %d blocks", peer, len(blockPack.blocks)) - if atomic.LoadInt32(&d.processing) == 0 { - go d.process() - } + go d.process() case errInvalidChain: // The hash chain is invalid (blocks are not ordered properly), abort @@ -701,6 +699,19 @@ func (d *Downloader) banBlocks(peerId string, head common.Hash) error { } // process takes blocks from the queue and tries to import them into the chain. +// +// The algorithmic flow is as follows: +// - The `processing` flag is swapped to 1 to ensure singleton access +// - The current `cancel` channel is retrieved to detect sync abortions +// - Blocks are iteratively taken from the cache and inserted into the chain +// - When the cache becomes empty, insertion stops +// - The `processing` flag is swapped back to 0 +// - A post-exit check is made whether new blocks became available +// - This step is important: it handles a potential race condition between +// checking for no more work, and releasing the processing "mutex". In +// between these state changes, a block may have arrived, but a processing +// attempt denied, so we need to re-enter to ensure the block isn't left +// to idle in the cache. func (d *Downloader) process() (err error) { // Make sure only one goroutine is ever allowed to process blocks at once if !atomic.CompareAndSwapInt32(&d.processing, 0, 1) { @@ -763,7 +774,7 @@ func (d *Downloader) process() (err error) { // Try to inset the blocks, drop the originating peer if there's an error index, err := d.insertChain(raw) if err != nil { - glog.V(logger.Debug).Infoln("Block #%d import failed:", raw[index].NumberU64(), err) + glog.V(logger.Debug).Infof("Block #%d import failed: %v", raw[index].NumberU64(), err) d.dropPeer(blocks[index].OriginPeer) d.Cancel() return errCancelChainImport -- cgit v1.2.3