Project

General

Profile

Actions

Bug #9494

closed

Race condition in autoload of Digest::SHA256, etc.

Added by Conrad.Irwin (Conrad Irwin) about 10 years ago. Updated over 9 years ago.

Status:
Closed
Target version:
-
[ruby-core:60527]

Description

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.


Files

require-digest-upfront.patch (1006 Bytes) require-digest-upfront.patch Conrad.Irwin (Conrad Irwin), 02/06/2014 07:51 AM

Updated by knu (Akinori MUSHA) over 9 years ago

  • 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?

Updated by knu (Akinori MUSHA) over 9 years ago

  • 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
Actions

Also available in: Atom PDF

Like0
Like0Like0