We should check that the C++ script implementation (appears to be) threadsafe, in order to be able to do parallel script verification (#868).
starting this now. cc @str4d @gtank incase either of you have any suggestions for things to double check.
So far I've checked the main logic, mostly in interpreter.cpp. That code seems to have no shared data beyond static consts which should be safe to share. Same goes for the calling logic in zcashconsensus.cpp/h. I'm going to go ahead and double check all of the .cpp files that are needed by the build.rs file in zcash_script.
pubkey.cpp looks like its likely not threadsafe.
namespace
{
/* Global secp256k1_context object used for verification. */
secp256k1_context* secp256k1_context_verify = NULL;
}
Shared global verification context that is mutated as far as I can tell X_X
Okay so upon further review it looks like it is thread safe, here's how:
So the secp context pointer is managed by this ECCVerifyHandle type that makes it act like a lazy_static:
/* static */ int ECCVerifyHandle::refcount = 0;
ECCVerifyHandle::ECCVerifyHandle()
{
if (refcount == 0) {
assert(secp256k1_context_verify == NULL);
secp256k1_context_verify = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY);
assert(secp256k1_context_verify != NULL);
}
refcount++;
}
ECCVerifyHandle::~ECCVerifyHandle()
{
refcount--;
if (refcount == 0) {
assert(secp256k1_context_verify != NULL);
secp256k1_context_destroy(secp256k1_context_verify);
secp256k1_context_verify = NULL;
}
}
This handle type is only ever constructed in one place, within zcash_script.cpp
struct ECCryptoClosure
{
ECCVerifyHandle handle;
};
ECCryptoClosure instance_of_eccryptoclosure;
This is a global variable that creates one of those EccVerifyHandles upon startup of the binary, which initializes the secp context pointer. This is the last time that global pointer is ever mutated, all usages of this context by zcash_script_verify are read only, which should be safe from multiple threads concurrently.