Address possible race condition in AsyncRPCQueue

[daira]

https://github.com/zcash/zcash/pull/1274/files#r78090033

@bitcartel:

I have added a lock for setting/getting member variables like error, result, etc. Not using the lock are:

state_ which is atomic. id_ and creation_time_ because they are only set at constructor time and should never be mutated. Have updated comment/code to clarify this for anyone looking to subclass and are wondering why they are private and not protected.

see commit [8a30b53]

@daira:

I believe you're assuming a "safe publication" property (https://shipilev.net/blog/2014/safe-public-construction/#_safe_publication) that does not actually hold in the C++ memory model. (There's actually very little discussion of this specifically in the context of C++; there's much more about it in the context of Java's memory model which does guarantee it.)

@daira:

On reflection this is probably not exploitable given typical implementations of mutexes on x86, which has a fairly strong memory model. Let's leave it as it is for beta 1, and I can do more research on how to fix it robustly.

[nathan-at-least]

We believe this will be rare or impossible for users on our target platforms at release, so we'll address it after release. (It may be dangerous.)

[nathan-at-least]

Removed 'needs prioritization' because this is already in a milestone.