Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Concurrent bundle install runs raise errors unlinking ~/.bundle/cache/compact_index/rubygems.org.443.[..]/versions #4519

Closed
domcleal opened this issue May 3, 2016 · 0 comments

Comments

@domcleal
Copy link
Contributor

domcleal commented May 3, 2016

  • What did you do?

    I ran the command bundle install in four parallel processes using different RVM gemsets (therefore different GEM_HOME), but under the same user account with a shared HOME environment variable. No Gemfile.lock was present, it was a clean checkout of a project with only a Gemfile.

  • What did you expect to happen?

    I expected Bundler to complete without error by either locking the files it's downloading to be safer with multiple processes, or to use caches that are unique per GEM_HOME.

  • What happened instead?

    Instead, what actually happened was one of the bundle install commands failed intermittently (a very small percentage of the time) with an error deleting ~/.bundle/cache/compact_index/rubygems.org.443.29b0360b937aa4d161703e6160654e47/versions.

    It appears that the compact index cache is shared for a whole user account without any locking, so multiple processes can try to download the same file at the same time and cause errors for each other.

Error details

Errno::ENOENT: No such file or directory - /home/jenkins/.bundle/cache/compact_index/rubygems.org.443.29b0360b937aa4d161703e6160654e47/versions
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/vendor/compact_index_client/lib/compact_index_client/updater.rb:50:in `unlink'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/vendor/compact_index_client/lib/compact_index_client/updater.rb:50:in `delete'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/vendor/compact_index_client/lib/compact_index_client/updater.rb:50:in `update'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/vendor/compact_index_client/lib/compact_index_client.rb:64:in `update'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/vendor/compact_index_client/lib/compact_index_client.rb:55:in `update_and_parse_checksums!'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/fetcher/compact_index.rb:63:in `available?'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/fetcher/compact_index.rb:15:in `call'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/fetcher/compact_index.rb:15:in `block in compact_index_request'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/fetcher.rb:154:in `use_api'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/source/rubygems.rb:331:in `block in api_fetchers'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/source/rubygems.rb:331:in `select'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/source/rubygems.rb:331:in `api_fetchers'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/source/rubygems.rb:336:in `block in remote_specs'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/index.rb:10:in `build'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/source/rubygems.rb:335:in `remote_specs'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/source/rubygems.rb:82:in `specs'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/definition.rb:211:in `block (2 levels) in index'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/definition.rb:209:in `each'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/definition.rb:209:in `block in index'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/index.rb:10:in `build'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/definition.rb:206:in `index'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/definition.rb:200:in `resolve'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/definition.rb:140:in `specs'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/definition.rb:129:in `resolve_remotely!'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/installer.rb:195:in `resolve_if_need'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/installer.rb:70:in `run'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/installer.rb:22:in `install'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/cli/install.rb:102:in `run'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/cli.rb:175:in `install'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/vendor/thor/lib/thor.rb:359:in `dispatch'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/vendor/thor/lib/thor/base.rb:440:in `start'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/cli.rb:11:in `start'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/exe/bundle:27:in `block in <top (required)>'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/lib/bundler/friendly_errors.rb:98:in `with_friendly_errors'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/gems/bundler-1.12.1/exe/bundle:19:in `<top (required)>'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/bin/bundle:23:in `load'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/bin/bundle:23:in `<main>'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/bin/ruby_executable_hooks:15:in `eval'
  /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/bin/ruby_executable_hooks:15:in `<main>'

Environment

Bundler 1.12.1
Rubygems 2.4.8
Ruby 2.0.0p353 (2013-11-22 revision 43784) [x86_64-linux]
GEM_HOME /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0
GEM_PATH /usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0:/usr/local/rvm/gems/ruby-2.0.0-p353@global
RVM 1.26.11 (1.26.11)
Git 1.8.3.1
rubygems-bundler (1.4.4)

  Bundler settings

orig_path
Set via BUNDLE_ORIG_PATH: "/usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0/bin:/usr/local/rvm/gems/ruby-2.0.0-p353@global/bin:/usr/local/rvm/rubies/ruby-2.0.0-p353/bin:/usr/local/rvm/bin:/usr/local/bin:/usr/bin"
orig_gem_path
Set via BUNDLE_ORIG_GEM_PATH: "/usr/local/rvm/gems/ruby-2.0.0-p353@test_kafo_parsers_master_pull_request-0:/usr/local/rvm/gems/ruby-2.0.0-p353@global"

@coilysiren coilysiren added this to the 1.X -- Index Format milestone May 4, 2016
domcleal added a commit to domcleal/bundler that referenced this issue May 10, 2016
When bundler is run concurrently using the same bundle dir in $HOME,
the versions file can be updated from two processes at once. The
download has been changed to a temporary file, which is securely moved
into place over the original.

If retrying the update operation, the original file is no longer
immediately deleted and instead a full download is performed, later
overwriting the original file if successful.

If two processes are updating in parallel, this should ensure the
original file isn't corrupted and that both processes succeed.

- Fixes rubygems#4519
domcleal added a commit to domcleal/bundler that referenced this issue May 10, 2016
When bundler is run concurrently using the same bundle dir in $HOME,
the versions file can be updated from two processes at once. The
download has been changed to a temporary file, which is securely moved
into place over the original.

If retrying the update operation, the original file is no longer
immediately deleted and instead a full download is performed, later
overwriting the original file if successful.

If two processes are updating in parallel, this should ensure the
original file isn't corrupted and that both processes succeed.

- Fixes rubygems#4519
domcleal added a commit to domcleal/bundler that referenced this issue May 10, 2016
When bundler is run concurrently using the same bundle dir in $HOME,
the versions file can be updated from two processes at once. The
download has been changed to a temporary file, which is securely moved
into place over the original.

If retrying the update operation, the original file is no longer
immediately deleted and instead a full download is performed, later
overwriting the original file if successful.

If two processes are updating in parallel, this should ensure the
original file isn't corrupted and that both processes succeed.

- Fixes rubygems#4519
domcleal added a commit to domcleal/bundler that referenced this issue May 10, 2016
When bundler is run concurrently using the same bundle dir in $HOME,
the versions file can be updated from two processes at once. The
download has been changed to a temporary file, which is securely moved
into place over the original.

If retrying the update operation, the original file is no longer
immediately deleted and instead a full download is performed, later
overwriting the original file if successful.

If two processes are updating in parallel, this should ensure the
original file isn't corrupted and that both processes succeed.

- Fixes rubygems#4519
homu added a commit that referenced this issue May 15, 2016
Safely store concurrent compact index downloads

When bundler is run concurrently using the same bundle dir in $HOME,
the versions file can be updated from two processes at once. The
download has been changed to a temporary file, which is securely moved
into place over the original.

If retrying the update operation, the original file is no longer
immediately deleted and instead a full download is performed, later
overwriting the original file if successful.

If two processes are updating in parallel, this should ensure the
original file isn't corrupted and that both processes succeed.

- Fixes #4519

---

This would be useful on 1.12.x if possible, since the new caching behaviour with a shared home directory is causing the errors described in #4519.
segiddins pushed a commit that referenced this issue May 16, 2016
Safely store concurrent compact index downloads

When bundler is run concurrently using the same bundle dir in $HOME,
the versions file can be updated from two processes at once. The
download has been changed to a temporary file, which is securely moved
into place over the original.

If retrying the update operation, the original file is no longer
immediately deleted and instead a full download is performed, later
overwriting the original file if successful.

If two processes are updating in parallel, this should ensure the
original file isn't corrupted and that both processes succeed.

- Fixes #4519

---

This would be useful on 1.12.x if possible, since the new caching behaviour with a shared home directory is causing the errors described in #4519.
indirect added a commit that referenced this issue Oct 3, 2016
As we noticed in #4519, we need to use a temporary directory to hold
compact index downloads so that multiple processes don't write to the
same files at the same time and break everything.

The fix for that was #4561, which added temporary directories to hold
all files as they download, and then uses the (atomic) `FileUtils.cp` to
move the completed downloads into place, so there is never a point where
multiple processes are trying to write into the file at once.

Unfortunately, using `Dir.mktmpdir` requires that the parent directory
be _either_ world writable or sticky, but not both. Based on #4599, it
looks like it's common for home directories to be both world writable
and sticky. While that's a security problem by itself, it's not a big
concern for Bundler and the compact index. So we want to let users
continue to use Bundler, even with the compact index, without having to
change the permissions on their home directories.

This commit changes the `mktmpdir` call to create the temporary
directory inside the default OS tempdir, which is typically `/tmp` or
`/var/tmp` depending on distro. Since that directory is designed to hold
other temporary directories, that change should (theoretically) reduce
or eliminate the problem reported in #4599.
homu added a commit that referenced this issue Oct 4, 2016
use /tmp for mktmpdir

As we noticed in #4519, we need to use a temporary directory to hold
compact index downloads so that multiple processes don't write to the
same files at the same time and break everything.

The fix for that was #4561, which added temporary directories to hold
all files as they download, and then uses the (atomic) `FileUtils.cp` to
move the completed downloads into place, so there is never a point where
multiple processes are trying to write into the file at once.

Unfortunately, using `Dir.mktmpdir` requires that the parent directory
be _either_ world writable or sticky, but not both. Based on #4599, it
looks like it's common for home directories to be both world writable
and sticky. While that's a security problem by itself, it's not a big
concern for Bundler and the compact index. So we want to let users
continue to use Bundler, even with the compact index, without having to
change the permissions on their home directories.

This commit changes the `mktmpdir` call to create the temporary
directory inside the default OS tempdir, which is typically `/tmp` or
`/var/tmp` depending on distro. Since that directory is designed to hold
other temporary directories, that change should (theoretically) reduce
or eliminate the problem reported in #4599.
indirect pushed a commit that referenced this issue Oct 11, 2016
use /tmp for mktmpdir

As we noticed in #4519, we need to use a temporary directory to hold
compact index downloads so that multiple processes don't write to the
same files at the same time and break everything.

The fix for that was #4561, which added temporary directories to hold
all files as they download, and then uses the (atomic) `FileUtils.cp` to
move the completed downloads into place, so there is never a point where
multiple processes are trying to write into the file at once.

Unfortunately, using `Dir.mktmpdir` requires that the parent directory
be _either_ world writable or sticky, but not both. Based on #4599, it
looks like it's common for home directories to be both world writable
and sticky. While that's a security problem by itself, it's not a big
concern for Bundler and the compact index. So we want to let users
continue to use Bundler, even with the compact index, without having to
change the permissions on their home directories.

This commit changes the `mktmpdir` call to create the temporary
directory inside the default OS tempdir, which is typically `/tmp` or
`/var/tmp` depending on distro. Since that directory is designed to hold
other temporary directories, that change should (theoretically) reduce
or eliminate the problem reported in #4599.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants