Project

General

Profile

Bug #9494

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

Added by Conrad.Irwin (Conrad Irwin) over 5 years ago. Updated almost 5 years ago.

Status:
Closed
Priority:
Normal
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

Associated revisions

Revision c02fa394
Added by knu (Akinori MUSHA) almost 5 years ago

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

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@48213 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

Revision 48213
Added by knu (Akinori MUSHA) almost 5 years ago

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

Revision 48213
Added by knu (Akinori MUSHA) almost 5 years ago

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

Revision 48213
Added by knu (Akinori MUSHA) almost 5 years ago

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

Revision 48213
Added by knu (Akinori MUSHA) almost 5 years ago

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

Revision 48213
Added by knu (Akinori MUSHA) almost 5 years ago

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

Revision 48213
Added by knu (Akinori MUSHA) almost 5 years ago

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

History

Updated by knu (Akinori MUSHA) almost 5 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) almost 5 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

Also available in: Atom PDF