-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Thread-unsafe class caching #27
Comments
@knu I see that you are now the maintainer of this project. This is still an issue with edit: I've opened a PR to fix this at #43. |
Dynamically requiring implementations at runtime in this way is not safe in a multithreaded program, even in MRI with the GIL. We can simplify this by just using autoload and turning the @@class_map into a simple case statement. Fixes sparklemotion#27
Dynamically requiring implementations at runtime in this way is not safe in a multithreaded program, even in MRI with the GIL. We can simplify this by just using autoload and turning the @@class_map into a simple case statement. Fixes sparklemotion#27
Hello @knu |
The PR looks fine to me but I am not an expert on this library. |
We discovered a bug in http-cookie while investigating jruby/jruby#5910.
In
AbstractStore
, (and also inAbstractSaver
I believe) you have code like this:This is not thread-safe in any implementation and is the true cause of our variable object widths.
The problem here is that the
inherited
hook is called immediately once theHashStore
and other subclasses extendAbstractStore
. In other words, the class goes into this@@class_map
before any of its methods have been defined! As a result, another thread might see and try to instantiate the class before it is fully initialized.This is the reason we had unpredictable results in jruby/jruby#5910; depending on when the second thread starts inspecting the class, it will see none, some, or all of
HashStore
's methods defined, giving us a different view of instance variables.It actually gets worse, though, because it's possible for a second thread to start instantiating
HashStore
before it is fully defined, which causesAbstractStore
's initialize to call the default version ofdefault_options
that returns nil, and you will get errors like this:The text was updated successfully, but these errors were encountered: