Fix race condition for multi-thread support #702
Conversation
|
I'm not very open to merge this PR. There are similar code in a lot of classes in RBS, and I don't want to make/keep everything thread safe. (My opinion is non-immutable objects in RBS are not thread-safe.) Leaving possible technical improvements. |
memoization This error manifests depending of ordering of signature validations, where modules can be reentered in before cache is assigned. Assuming that the result of it is idempotent, it shouldn't be an issue.
HoneyryderChuck
force-pushed
the
issue-695
branch
from
83f25e1 to
4e67199
Compare
|
@soutaro removed the lock strategies. I found out during the testing that the issue is not thread-safety, but rather the order in which the modules are saved and cached, i.e. a class signature might be building, where used modules might make a reference to it, before it ever gets cached. It's probably a tad inneficient, although I don't think it's an issue, assuming the result is idempotent. |
Oh no. It sounds reasonable.
Correct. Dose this patch solve your problem? |
|
It does. 👍 |
soutaro
changed the title
|
@HoneyryderChuck Merged. Thank you for finding and fixing the issue! 🎉 |
Inside DefinitionBuilder, access to caches happens recursively, where
set operations mark the field in the cache with false. This breaks
sometimes when used in a threaded context, such as a minitest parallel
run.
This patch fixes it by making cache accesses mutex-aware, while also
allowing recursive sets on the same cache.
Fixes #695