On 22.01.2016 19:47, Vincent Povirk wrote:
I actually think it's very important to get theading right, even if it's a "theoretical issue" that "probably won't happen".
A managed C++ project probably will not do its runtime initialization from multiple threads, but it could have multiple threads accessing another vtable later. Vtable fixups could also be used to export functions to a mostly unmanaged program that expects them to be thread-safe.
I still believe this function is safe, but if it's not then I want to address it.
Mono has a lock around the JIT compiler, and it caches the functions it generates. So we can have multiple threads writing the same function pointers into the vtable (which should be fine as long as every other thread that reads it will see either the old value or the new one). Mono doesn't reference-count JITed functions, and I'm not sure if cleaning them up is even possible.
Non-locked write to fixup->done is also OK as long as no thread can incorrectly read the value as TRUE and return before the vtable is ready. If a thread incorrectly reads it as FALSE, it'll just do a bit of unnecessary work. Actually, I'm not sure that field is needed at all, it seems like overwriting the values in the vtable should be enough.
Besides possible memory leaks of JITed functions, there is no real risk for race-conditions here. Various other parts of Wine also rely on the fact that values up to the size of a pointer can be written with one memory access,