diff options
author | David RodrÃguez <deivid.rodriguez@riseup.net> | 2021-02-25 18:43:51 +0100 |
---|---|---|
committer | git <svn-admin@ruby-lang.org> | 2021-11-30 20:54:05 +0900 |
commit | 7fd88da935c7c6fcafe19cf30642676033ec82bd (patch) | |
tree | 4ad6409f708d7f80f368045c187fd8782c9cde75 /lib/rubygems | |
parent | d7f6cb0f780a5a48b5d4a937f93d876a90697fc0 (diff) |
[rubygems/rubygems] Fix race condition when reading & writing gemspecs concurrently
When bundler parallel installer installs gems concurrently, one can get
confusing warnings like the following:
```
"[/home/runner/work/rubygems/rubygems/bundler/tmp/2/gems/system/specifications/zeitwerk-2.4.2.gemspec] isn't a Gem::Specification (NilClass instead).
```
I've got these warnings several times in the past, but I never managed
to reproduce them, and never look deeply into the root cause, but this
time a got a cause that reproduced quite frequently, so I looked into
it.
The problem is one thread reading a gemspec while another thread is
writing it. The write of the gemspec was not protected, so
`Gem::Specification.load` could end up seeing a truncated gemspec and
thus throw this warning.
The fix involve two changes:
* Change the methods that write gemspecs to use `Gem.binary_write` which
is protected by a lock.
* Fix `Gem.binary_write` to create the file lock at file creation time,
not when the file already exists after.
The realworld user problem caused by this issue happens in bundler, but
I'm fixing it in rubygems first, and then I'll backport to bundler
whatever needs backporting to fix the issue on the bundler side.
https://github.com/rubygems/rubygems/commit/a672e7555c
Diffstat (limited to 'lib/rubygems')
-rw-r--r-- | lib/rubygems/installer.rb | 12 |
1 files changed, 3 insertions, 9 deletions
diff --git a/lib/rubygems/installer.rb b/lib/rubygems/installer.rb index 38642ee8ef..8e3965ef92 100644 --- a/lib/rubygems/installer.rb +++ b/lib/rubygems/installer.rb @@ -446,13 +446,9 @@ class Gem::Installer # specifications directory. def write_spec - File.open spec_file, 'w' do |file| - spec.installed_by_version = Gem.rubygems_version + spec.installed_by_version = Gem.rubygems_version - file.puts spec.to_ruby_for_cache - - file.fsync rescue nil # for filesystems without fsync(2) - end + Gem.write_binary(spec_file, spec.to_ruby_for_cache) end ## @@ -460,9 +456,7 @@ class Gem::Installer # specifications/default directory. def write_default_spec - File.open(default_spec_file, "w") do |file| - file.puts spec.to_ruby - end + Gem.write_binary(default_spec_file, spec.to_ruby) end ## |