eth, les: wait for the downloads to be complete before stopping

[mandrigin]

When stopping a node, and the list of headers to rollback is full (in downloader.go:1160) the database might be closed before the rollback changes are written there.

That causes the crashes like status-im/status-go#652 and might be a source of a potential data loss/corruption.

This PR adds a method to wait for the downloads to be actually processed before proceeding forward and closing the DB.

[karalabe]

Hmm, good catch on the issue. Regarding the solution, I'm wondering if we could reuse the already present downloader.Cancel and downloader.Terminate mechanisms to have those properly wait instead of introducing further new methods?

[mandrigin]

@karalabe Yeah, that is a good idea.
Though, downloader.Terminate() is being called from a different goroutine than protocolManager.Stop() (syncer in les/handler.go:224).

I guess I could just move pm.downloader.Terminate() from syncer() to Stop().

Does that sound like a better approach?

UPD: and waiting should be done in downloader.Cancel then, as I understand
UPD2: putting it in downloader.Cancel deadlocks, so I'll put it in Terminate.

[mandrigin]

@karalabe updated the PR. Terminate now blocks until the downloads are finished.

[mandrigin]

@karalabe is there anything else that needs to be changed in this change?

[mandrigin]

@karalabe is there anything else needs to be fixed in this review?
we have a patch for that in our code, but I'll be glad to see this fixed upstream too...

[mandrigin]

@karalabe I just wanted to follow-up on this specific PR. It's been a while and we (at Status) still have this patch to keep our CI stable. It would be amazing to get more feedback on it and, if possible, to merge it upstream, so we can have one patch less for LES 😆

[holiman]

This PR touches some fairly "deep" internal stuff, hence why it's taking so long. Someone needs to review it properly, which will take quite some time.
One thing that would help is if you could create a test which reproduces the original bug.

[fjl]

The underlying issue was finally resolved for eth.Ethereum in this recent PR: #20695
Still need to fix it for LES.

[fjl]

Closed this now because the PR was quite old and les has changed a lot since it was opened.