pool: Correct racy paymgr processing logic.

[davecgh]

This corrects the mutex logic in the payment manager that keeps track of whatever or not payments are already being processed.

In particular, the code that is being updated previously checked and set the processing flag under two separate locks of the mutex which could easily result in the first check returning false for two concurrent goroutines followed both both setting it to true and executing which would defeat the purpose of the check.

[jholdstock]

Given that payDividends is the only func which accesses the Mutex, I think this can be really simplified. Mutex can just be locked once at the start of the func and then unlocked in a defer.

I don't even think its actually possible for payDividends to be called concurrently because it only has one caller which is in a single goroutine, but we can explore that in another PR.

[davecgh]

Given that payDividends is the only func which accesses the Mutex, I think this can be really simplified. Mutex can just be locked once at the start of the func and then unlocked in a defer.

I originally changed it that way, as I also highly suspect that's true, however I haven't dug into the call paths yet and didn't want to change the semantics until I verify, so I opted for this approach.

[davecgh]

Looking a little more closely, I see that that payDividends is the result of receiving a message on the paymentCh which itself is sent via processPayments. Then processPayments is used in the chain state config and is invoked as a goroutine from handleChainUpdates for connected blocks.

Since blocks can arrive at any time, and it's executed as a goroutine, it is possible that payDividends is still running. Changing it to a mutex for the entire function would cause it to end up running the entirety of payDividends for every connected block instead of intentionally skipping rapid blocks and processing them as a batch as it does with the current semantics.