Fixed panic with dHelper. #4562
Conversation
|
it would be good if you can explain the issue and how this PR fixes it |
| @@ -19,12 +19,13 @@ type downloader interface { | |||
| // Set downloader set the downloader of the shard to consensus | |||
| // TODO: It will be better to move this to consensus.New and register consensus as a service | |||
| func (consensus *Consensus) SetDownloader(d downloader) { | |||
| consensus.mutex.Lock() | |||
| defer consensus.mutex.Unlock() | |||
| consensus.dHelper = newDownloadHelper(consensus, d) | |||
There was a problem hiding this comment.
This is where we initialize dHelper
| @@ -104,7 +101,7 @@ func (consensus *Consensus) AddConsensusLastMile() error { | |||
| } | |||
|
|
|||
| func (consensus *Consensus) spinUpStateSync() { | |||
| consensus.dHelper.d.DownloadAsync() | |||
|
Use of dHelper can happen before it initialization. |
| consensus.mutex.Lock() | ||
| defer consensus.mutex.Unlock() |
There was a problem hiding this comment.
could you explain why we need this here ?
There was a problem hiding this comment.
this code is called from function BlockCommitSigs which is running in different goroutine. Without this lock it's going to be a race condition, because FBFTLog, which is called later, is not thread safe.
Could you explain how at this issue happen. You have a fix here which is great, but i don't get how this issue happens at the first place |
We had checks like |
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1a973f5]
goroutine 5441 [running]:
github.com/harmony-one/harmony/consensus.(*Consensus).spinUpStateSync(0xc00017a1e0)
/home/sp/harmony/harmony/consensus/downloader.go:107 +0x35
github.com/harmony-one/harmony/consensus.(*Consensus).onCommitted(0xc00017a1e0, 0xc008e3cfc0)