Skip to content

Conversation

@fwitzke
Copy link

@fwitzke fwitzke commented Dec 2, 2025

Summary

Fixes a race condition in Builder#build that causes NoMethodError: undefined method '[]' for nil:NilClass when multiple threads concurrently access uploader versions for the first time.

Problem

The Builder class uses lazy initialization for version classes. Without synchronization, concurrent threads can observe a partially initialized @klass where the class is created but version_options is not yet set:

def build(superclass)
  return @klass if @klass           # Thread B checks here, sees truthy @klass
  @klass = Class.new(superclass)    # Thread A just set this
  # ... Thread B returns partially initialized @klass ...
  @klass.version_options = @options # Thread A hasn't reached here yet
end

When version_active? tries to access version_options[:if] on the partially initialized class, it fails:

NoMethodError: undefined method `[]' for nil:NilClass
  from lib/carrierwave/uploader/versions.rb:220:in `version_active?'

This manifests in multi-threaded environments (like Sidekiq workers) when multiple threads simultaneously access versions for the first time after a process starts.

Solution

Uses double-checked locking pattern with a Mutex:

  1. Quick check without lock (fast path for already-initialized case)
  2. If not initialized, acquire mutex and check again
  3. Build class to a local variable first
  4. Only assign to @klass after full initialization

This ensures other threads never see a partially initialized class.

Also updates deep_dup to create a new Mutex for duplicated builders, preventing shared mutex state across inheritance.

Test Plan

  • Added thread safety tests that verify concurrent access doesn't cause race conditions
  • All existing version specs pass (91 examples, 0 failures)

Related Issues

Related to issues involving race conditions or thread-safety in version building.

The Builder class uses lazy initialization for version classes. Without
synchronization, concurrent threads can observe a partially initialized
@klass where the class is created but version_options is not yet set.

This causes NoMethodError when version_active? tries to access
version_options[:if] on the partially initialized class:

  NoMethodError: undefined method `[]' for nil:NilClass
    from .../versions.rb:220:in `version_active?'

The fix uses double-checked locking pattern:
1. Quick check without lock (fast path for already-initialized case)
2. If not initialized, acquire mutex and check again
3. Build class to a local variable first
4. Only assign to @klass after full initialization

This ensures other threads never see a partially initialized class.

Also updates deep_dup to create a new Mutex for duplicated builders,
preventing shared mutex state across inheritance.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant