ruby-changes:61520
From: David <ko1@a...>
Date: Fri, 5 Jun 2020 07:33:55 +0900 (JST)
Subject: [ruby-changes:61520] 603edfcaa0 (master): [rubygems/rubygems] Fix parallel installer race condition
https://git.ruby-lang.org/ruby.git/commit/?id=603edfcaa0 From 603edfcaa0aa6ea6660d045194769046d24a59aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@r...> Date: Sat, 30 May 2020 17:37:56 +0200 Subject: [rubygems/rubygems] Fix parallel installer race condition The main thread may detect that installation has finished and thus abort, while the thread responsible for installing the spec that failed has not yet set the error message. In this case, the install process will abort with a misterious "empty" exception. I can be force the issue to reproduce by applying the following patch: ```diff diff --git a/bundler/lib/bundler/installer/parallel_installer.rb b/bundler/lib/bundler/installer/parallel_installer.rb index 3dee9f4664..7827a11d11 100644 --- a/bundler/lib/bundler/installer/parallel_installer.rb +++ b/bundler/lib/bundler/installer/parallel_installer.rb @@ -166,6 +166,7 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/bundler/lib/bundler/installer/parallel_installer.rb#L166 spec_install.post_install_message = message unless message.nil? else spec_install.state = :failed + sleep 1 spec_install.error = "#{message}\n\n#{require_tree_for_spec(spec_install.spec)}" end Plugin.hook(Plugin::Events::GEM_AFTER_INSTALL, spec_install) @@ -183,6 +184,7 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/bundler/lib/bundler/installer/parallel_installer.rb#L184 end def finished_installing? + sleep 0.5 @specs.all? do |spec| return true if spec.failed? spec.installed? diff --git a/bundler/lib/bundler/rubygems_gem_installer.rb b/bundler/lib/bundler/rubygems_gem_installer.rb index 8ce33c3953..c585cd517b 100644 --- a/bundler/lib/bundler/rubygems_gem_installer.rb +++ b/bundler/lib/bundler/rubygems_gem_installer.rb @@ -42,6 +42,7 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/bundler/lib/bundler/rubygems_gem_installer.rb#L42 return true unless source = @package.instance_variable_get(:@gem) return true unless source.respond_to?(:with_read_io) digest = source.with_read_io do |io| + raise BundlerError, "asdafss" digest = SharedHelpers.digest(:SHA256).new digest << io.read(16_384) until io.eof? io.rewind ``` and running `bin/rspec spec/install/gems/compact_index_spec.rb:892` will result in ``` Run options: include {:locations=>{"./spec/install/gems/compact_index_spec.rb"=>[892]}} exclude {:jruby=>true, :readline=>false, :permissions=>false, :no_color_tty=>false, :ruby_repo=>false, :bundler=>"!= 2", :git=>"!= 2.26.2", :rubygems=>"!= 3.2.0.pre1", :realworld=>true, :sudo=>true} Randomized with seed 59277 F Retried examples: 0 Failures: 1) compact index api checksum validation raises when the checksum is the wrong length Failure/Error: expect(err).to include("The given checksum for rack-1.0.0 (\"checksum!\") is not a valid SHA256 hexdigest nor base64digest") expected "" to include "The given checksum for rack-1.0.0 (\"checksum!\") is not a valid SHA256 hexdigest nor base64digest" Commands: $ /home/deivid/.rbenv/versions/2.7.1/bin/ruby -I/home/deivid/Code/rubygems/rubygems/bundler/spec -r/home/deivid/Code/rubygems/rubygems/bundler/spec/support/artifice/compact_index_wrong_gem_checksum.rb -r/home/deivid/Code/rubygems/rubygems/bundler/spec/support/hax.rb /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/bin/bundle install --verbose --retry 0 Running `bundle install --retry 0 --verbose` with bundler 2.2.0.dev Found changes from the lockfile, re-resolving dependencies because the list of sources changed, the dependencies in your gemfile changed, you added a new platform to your gemfile HTTP GET http://localgemserver.test/versions HTTP 200 OK http://localgemserver.test/versions Fetching gem metadata from http://localgemserver.test/ Looking up gems ["rack"] HTTP GET http://localgemserver.test/info/rack HTTP 200 OK http://localgemserver.test/info/rack Resolving dependencies... Using bundler 2.2.0.dev 0: bundler (2.2.0.dev) from /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/specifications/bundler-2.2.0.dev.gemspec Fetching rack 1.0.0 Installing rack 1.0.0 Bundler::InstallError: /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/installer/parallel_installer.rb:199:in `handle_error' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/installer/parallel_installer.rb:102:in `call' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/installer/parallel_installer.rb:78:in `call' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/installer.rb:271:in `install_in_parallel' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/installer.rb:197:in `install' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/installer.rb:92:in `block in run' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/process_lock.rb:12:in `block in lock' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/process_lock.rb:9:in `open' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/process_lock.rb:9:in `lock' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/installer.rb:73:in `run' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/installer.rb:25:in `install' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/cli/install.rb:66:in `run' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/cli.rb:261:in `block in install' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/settings.rb:121:in `temporary' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/cli.rb:260:in `install' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/vendor/thor/lib/thor.rb:399:in `dispatch' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/cli.rb:30:in `dispatch' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/vendor/thor/lib/thor/base.rb:476:in `start' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/cli.rb:24:in `start' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/exe/bundle:49:in `block in <top (required)>' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/lib/bundler/friendly_errors.rb:117:in `with_friendly_errors' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/gems/bundler-2.2.0.dev/exe/bundle:37:in `<top (required)>' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/bin/bundle:23:in `load' /home/deivid/Code/rubygems/rubygems/bundler/tmp/1/gems/system/bin/bundle:23:in `<main>' # $? => 5 # ./spec/install/gems/compact_index_spec.rb:892:in `block (3 levels) in <top (required)>' # ./spec/spec_helper.rb:104:in `block (4 levels) in <top (required)>' # ./spec/spec_helper.rb:104:in `block (3 levels) in <top (required)>' # ./spec/support/helpers.rb:352:in `block in with_gem_path_as' # ./spec/support/helpers.rb:366:in `without_env_side_effects' # ./spec/support/helpers.rb:348:in `with_gem_path_as' # ./spec/spec_helper.rb:101:in `block (2 levels) in <top (required)>' # ./spec/spec_helper.rb:73:in `block (2 levels) in <top (required)>' # ./spec/support/rubygems_ext.rb:90:in `load' # ./spec/support/rubygems_ext.rb:90:in `gem_load_and_activate' # ./spec/support/rubygems_ext.rb:18:in `gem_load' Finished in 3.01 seconds (files took 0.14209 seconds to load) 1 example, 1 failure Failed examples: rspec ./spec/install/gems/compact_index_spec.rb:886 # compact index api checksum validation raises when the checksum is the wrong length Randomized with seed 59277 ``` Without any mention to `BundlerError` and the original "asdafss" message. Fix the issue by making sure the error message is set before the ":failed" status. https://github.com/rubygems/rubygems/commit/83c8feb2c4 diff --git a/lib/bundler/installer/parallel_installer.rb b/lib/bundler/installer/parallel_installer.rb index 3dee9f4..ad34e4f 100644 --- a/lib/bundler/installer/parallel_installer.rb +++ b/lib/bundler/installer/parallel_installer.rb @@ -165,8 +165,8 @@ module Bundler https://github.com/ruby/ruby/blob/trunk/lib/bundler/installer/parallel_installer.rb#L165 spec_install.state = :installed spec_install.post_install_message = message unless message.nil? else - spec_install.state = :failed spec_install.error = "#{message}\n\n#{require_tree_for_spec(spec_install.spec)}" + spec_install.state = :failed end Plugin.hook(Plugin::Events::GEM_AFTER_INSTALL, spec_install) spec_install -- cgit v0.10.2 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/