Unsychronized update of shared mutable state #3367
Comments
|
I think something like the following should fix that: diff --git a/lib/bundler/definition.rb b/lib/bundler/definition.rb
index 0349af39f..5272becd8 100644
--- a/lib/bundler/definition.rb
+++ b/lib/bundler/definition.rb
@@ -270,7 +270,7 @@ module Bundler
end
double_check_for_index(idx, dependency_names)
- end
+ end.freeze
end
# Suppose the gem Foo depends on the gem Bar. Foo exists in Source A. Bar has some versions that exist in both
diff --git a/lib/bundler/index.rb b/lib/bundler/index.rb
index 9166a9273..33a0a47fb 100644
--- a/lib/bundler/index.rb
+++ b/lib/bundler/index.rb
@@ -21,15 +21,15 @@ module Bundler
def initialize
@sources = []
@cache = {}
- @specs = Hash.new {|h, k| h[k] = {} }
- @all_specs = Hash.new {|h, k| h[k] = EMPTY_SEARCH }
+ @specs = {}
+ @all_specs = {}
end
def initialize_copy(o)
@sources = o.sources.dup
@cache = {}
- @specs = Hash.new {|h, k| h[k] = {} }
- @all_specs = Hash.new {|h, k| h[k] = EMPTY_SEARCH }
+ @specs = {}
+ @all_specs = {}
o.specs.each do |name, hash|
@specs[name] = hash.dup
@@ -39,6 +39,13 @@ module Bundler
end
end
+ def freeze
+ @sources.each(&:freeze).freeze
+ @specs.freeze
+ @all_specs.freeze
+ super
+ end
+
def inspect
"#<#{self.class}:0x#{object_id} sources=#{sources.map(&:inspect)} specs.size=#{specs.size}>"
end
@@ -49,7 +56,7 @@ module Bundler
end
def search_all(name)
- all_matches = local_search(name) + @all_specs[name]
+ all_matches = local_search(name) + (@all_specs[name] || EMPTY_SEARCH)
@sources.each do |source|
all_matches.concat(source.search_all(name))
end
@@ -102,7 +109,7 @@ module Bundler
alias_method :[], :search
def <<(spec)
- @specs[spec.name][spec.full_name] = spec
+ (@specs[spec.name] ||= {})[spec.full_name] = spec
spec
end
@@ -182,7 +189,11 @@ module Bundler
private
def specs_by_name(name)
- @specs[name].values
+ if s = @specs[name]
+ s.values
+ else
+ [] # must return a mutable array for compatibility with the other branch
+ end
end
def search_by_dependency(dependency, base = nil)
@@ -206,7 +217,8 @@ module Bundler
EMPTY_SEARCH = [].freeze
def search_by_spec(spec)
- spec = @specs[spec.name][spec.full_name]
+ versions = @specs[spec.name]
+ spec = versions && versions[spec.full_name]
spec ? [spec] : EMPTY_SEARCH
end
end
|
|
I consider Rubinius no longer active, so closing. |
|
Rubinius is working just fine and this is a credible report of a concurrency bug in Bundler. Fairly recently, I significantly updated the object allocation mechanisms to make it possible to track when threads are modifying objects the thread did not create (or has not been given ownership of). I’ll be adding more capabilities and tooling around this because nothing really exists for debugging concurrency issues in the Ruby space. For reference, what is your guidance on reporting concurrency bugs in Bundler? Are you simply unconcerned about that class of bug? |
|
Definitely a bug we should fix. |
|
Is this reproducible these days? |
|
Closing due to lack of feedback. |
Error Report
Questions
Please fill out answers to these questions, it'll help us figure out
why things are going wrong.
What did you do?
I ran the command
/source/rubinius/rubinius/build/rubinius/gems/bin/bundle install --jobs=40in a new Rails 5 app.What did you expect to happen?
I expected Bundler to complete the install.
What happened instead?
Instead, what happened was the install failed with the exception listed below.
Have you tried any solutions posted on similar issues in our issue tracker, stack overflow, or google?
I examined the backtrace and noted that the following code
@all_specs = Hash.new {|h, k| h[k] = EMPTY_SEARCH }located inlib/bundler/index.rb:32updates the Hash instance with no synchronization.Have you read our issues document, https://github.com/bundler/bundler/blob/master/doc/contributing/ISSUES.md?
Yes.
Backtrace
Environment
Bundler Build Metadata
Bundler settings
Gemfile
Gemfile
Gemfile.lock
The text was updated successfully, but these errors were encountered: