Bug #9494: Race condition in autoload of Digest::SHA256, etc.

• Status: Closed
• Assignee: knu (Akinori MUSHA)
• Target version: -
• ruby -v: 2.0.0p247
• Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN, 2.1: UNKNOWN

[Conrad.Irwin (Conrad Irwin)]

At the moment the digest extension uses const_missing to implement autoload. Unfortunately there's a race condition: another thread can run after the constant is defined, but before it is initialized. (http://git.io/YW3bbA)

There's a minimal test case here: http://git.io/5umBqQ — this was happening to us regularly in production because each sidekiq worker would race to create a hash.

The work around we have deployed is to require digest/sha2.so pre-emptively. And the attached patch does that.

An alternative solution would be to use autoload, but that is deprecated; or to change the way meta-data is stored for Digest classes so that it can be accessed immediately. There's no way to use a mutex inside const_missing to fix this, because the bug is that the constant becomes not-missing before it is fully defined.

[knu (Akinori MUSHA)]

• Status changed from Open to Assigned
• Assignee set to knu (Akinori MUSHA)

Hi,

Thanks for the patch, but this change is hardly acceptable because it unconditionally increases memory consumption just for solving a problem under a multi-threaded environment.

The problem is unavoidable as long as you rely on a constant lookup, because constants defined in a module becomes immediately visible to other threads and there is virtually no way to prevent other threads that refer to the constant from using it before the module finishes initialization.

One way to fix it is preload 'digest/*' on application boot just as you said, but I can imagine there will be a situation where this is not acceptable. So, to mitigate the problem under a multi-threaded environment, I'm going to turn Digest() into a thread-safe function by making Digest(:FOO) always call require 'digest/foo' before returning Digest::FOO. This is for use in a situation when threads are involved. You say Digest(:FOO) instead of Digest::FOO and it's thread-safe.

What do you think?

[knu (Akinori MUSHA)]

• Status changed from Assigned to Closed
• % Done changed from 0 to 100

Applied in changeset r48213.

---

Make Digest() thread-safe.

• ext/digest/lib/digest.rb (Digest()): This function should now be
  thread-safe. If you have a problem with regard to on-demand
  loading under a multi-threaded environment, preload "digest/"
  modules on boot or use this method instead of directly
  referencing Digest::. [Bug #9494]
  cf. https://github.com/aws/aws-sdk-ruby/issues/525