mapArgs is modified by setgenerate RPC without locking

[daira]

https://github.com/zcash/zcash/blob/master/src/rpcmining.cpp#L315-L316

    mapArgs["-gen"] = (fGenerate ? "1" : "0");
    mapArgs ["-genproclimit"] = itostr(nGenProcLimit);

@laanwj wrote here:

We should probably change this at some point too to not update the mapArgs. Throughout the program we assume that mapArgs is read-only after initialization. This kind of r/w access would need proper locking around everything.

The problem was present in Bitcoin before we forked.

[laanwj]

See bitcoin/bitcoin#9243

[nathan-at-least]

I left this in on the assumption the rebase from upstream is easy enough. Let's time box it.

[str4d]

Here's the chain of PRs that I've currently identified for merging (to reduce conflicts):

• Cache transaction validation successes bitcoin/bitcoin#6077
  • Second commit only (first was reverted)
• Fix argument parsing oddity with -noX bitcoin/bitcoin#6284
• Give a better error message if system clock is bad bitcoin/bitcoin#6489
• implement uacomment config parameter which can add comments to user agent as per BIP-0014 bitcoin/bitcoin#6462
• Sanitize uacomment bitcoin/bitcoin#6647
• Chainparams: Translations: DRY: options and error strings bitcoin/bitcoin#6235
• Use constants and minor fixes by luke-jr bitcoin/bitcoin#6905
• Call init's parameter interaction before we create the UI options model bitcoin/bitcoin#6780
  • Excluding second commit (QT) and third commit (requires Add -blocksonly option bitcoin/bitcoin#6993)
• luke-jr constants bitcoin/bitcoin#6961
  • Excluding QT parts, and a small src/policy/policy.cpp change which depends on a bunch of other PRs, we'll have to remember to come back to it.
• RPC: Added additional config option for multiple RPC users. bitcoin/bitcoin#7044
• Globals: Decouple GetConfigFile and ReadConfigFile from global mapArgs bitcoin/bitcoin#8856
• Make connect=0 disable automatic outbound connections. bitcoin/bitcoin#9002
• Clean up mapArgs and mapMultiArgs Usage bitcoin/bitcoin#9243
  • Excluding BIP 9 changes.

I think I have identified all the parents, but will update this list again if necessary. I have a branch with the current parents.

[str4d]

Another parent PR is bitcoin/bitcoin#6870 which itself should be merged after -maxmempool is added. As IIRC we have already discussed including that, I will do so in its own PR, so I will make a PR now with the earlier misc ones.

[nathan-at-least]

Bumped out of 1.0.9 for a few reasons: First I want to require more scrutiny of upstream PRs, and second because we're out of time for this issue.