Thread-unsafe class caching

[headius]

We discovered a bug in http-cookie while investigating jruby/jruby#5910.

In AbstractStore, (and also in AbstractSaver I believe) you have code like this:

...
  class << self
    @@class_map = {}

    # Gets an implementation class by the name, optionally trying to
    # load "http/cookie_jar/*_store" if not found.  If loading fails,
    # IndexError is raised.
    def implementation(symbol)
      @@class_map.fetch(symbol)
    rescue IndexError
      begin
        require 'http/cookie_jar/%s_store' % symbol
        @@class_map.fetch(symbol)
      rescue LoadError, IndexError
        raise IndexError, 'cookie store unavailable: %s' % symbol.inspect
      end
    end

    def inherited(subclass) # :nodoc:
      @@class_map[class_to_symbol(subclass)] = subclass
    end

    def class_to_symbol(klass) # :nodoc:
      klass.name[/[^:]+?(?=Store$|$)/].downcase.to_sym
    end
  end
...

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 the HashStore and other subclasses extend AbstractStore. 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 causes AbstractStore's initialize to call the default version of default_options that returns nil, and you will get errors like this:

...
NoMethodError: undefined method `each_pair' for nil:NilClass
  initialize at /Users/headius/projects/jruby/lib/ruby/gems/shared/gems/http-cookie-1.0.3/lib/http/cookie_jar/abstract_store.rb:52
    repro.rb at repro.rb:5

[brasic]

@knu I see that you are now the maintainer of this project. This is still an issue with JRuby and other ruby implementations that do not include a GIL all ruby implementations, requiring workarounds to avoid the lazy class loading. Would you be open to a pull request that fixes it by eagerly loading all CookieJar implementations at require time instead of lazily loading in a thread-unsafe way?

edit: I've opened a PR to fix this at #43.

[polyakovigor]

Hello @knu
Can you please provide the status for the issue highlighted here? I'm experiencing the same error.
Thanks

[headius]

The PR looks fine to me but I am not an expert on this library.