From a11fe57cd2c6b982288a8f7436e013fc53e8458b Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Thu, 20 Sep 2018 10:57:27 +0100 Subject: [PATCH] cache_store: handle corrupt DBM database. When the DBM database cannot be read by the current version of Ruby's DBM library (due to corruption or another incompatibility) it segfaults or freezes which takes down the entire Homebrew Ruby process. This isn't desirable so instead perform a shell out with the Homebrew Ruby to see if it can read the DBM database before we try to use the information. If this hangs or crashes: silently delete the database and recreate it. --- Library/Homebrew/cache_store.rb | 34 +++++++++++++++++++++-- Library/Homebrew/system_command.rb | 4 +++ Library/Homebrew/test/cache_store_spec.rb | 10 +++---- Library/Homebrew/utils/ruby.sh | 2 +- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/Library/Homebrew/cache_store.rb b/Library/Homebrew/cache_store.rb index 80232dd897..ff5ca0ad33 100644 --- a/Library/Homebrew/cache_store.rb +++ b/Library/Homebrew/cache_store.rb @@ -1,5 +1,6 @@ require "dbm" require "json" +require "timeout" # # `CacheStoreDatabase` acts as an interface to a persistent storage mechanism @@ -46,7 +47,7 @@ class CacheStoreDatabase # # @return [Boolean] def created? - File.exist?(cache_path) + cache_path.exist? end private @@ -57,6 +58,10 @@ class CacheStoreDatabase # https://docs.oracle.com/cd/E17276_01/html/api_reference/C/envopen.html DATABASE_MODE = 0664 + # Spend 5 seconds trying to read the DBM file. If it takes longer than this it + # has likely hung or segfaulted. + DBM_TEST_READ_TIMEOUT = 5 + # Lazily loaded database in read/write mode. If this method is called, a # database file with be created in the `HOMEBREW_CACHE` with name # corresponding to the `@type` instance variable @@ -66,6 +71,29 @@ class CacheStoreDatabase # DBM::WRCREAT: Creates the database if it does not already exist @db ||= begin HOMEBREW_CACHE.mkpath + if created? + dbm_test_read_cmd = SystemCommand.new( + ENV["HOMEBREW_RUBY_PATH"], + args: [ + "-rdbm", + "-e", + "DBM.open('#{dbm_file_path}', #{DATABASE_MODE}, DBM::READER).size", + ], + print_stderr: false, + must_succeed: true, + ) + dbm_test_read_success = begin + Timeout.timeout(DBM_TEST_READ_TIMEOUT) do + dbm_test_read_cmd.run! + true + end + rescue ErrorDuringExecution, Timeout::Error + odebug "Failed to read #{dbm_file_path}!" + Process.kill(:KILL, dbm_test_read_cmd.pid) + false + end + cache_path.delete unless dbm_test_read_success + end DBM.open(dbm_file_path, DATABASE_MODE, DBM::WRCREAT) end end @@ -83,7 +111,7 @@ class CacheStoreDatabase # # @return [String] def dbm_file_path - File.join(HOMEBREW_CACHE, @type.to_s) + "#{HOMEBREW_CACHE}/#{@type}" end # The path where the database resides in the `HOMEBREW_CACHE` for the given @@ -91,7 +119,7 @@ class CacheStoreDatabase # # @return [String] def cache_path - "#{dbm_file_path}.db" + Pathname("#{dbm_file_path}.db") end end diff --git a/Library/Homebrew/system_command.rb b/Library/Homebrew/system_command.rb index e53dd9bb66..29f3fdfcff 100644 --- a/Library/Homebrew/system_command.rb +++ b/Library/Homebrew/system_command.rb @@ -19,6 +19,8 @@ end class SystemCommand extend Predicable + attr_reader :pid + def self.run(executable, **options) new(executable, **options).run! end @@ -122,6 +124,7 @@ class SystemCommand raw_stdin, raw_stdout, raw_stderr, raw_wait_thr = Open3.popen3(env, [executable, executable], *args, **options) + @pid = raw_wait_thr.pid write_input_to(raw_stdin) raw_stdin.close_write @@ -191,6 +194,7 @@ class SystemCommand end def success? + return false if @exit_status.nil? @exit_status.zero? end diff --git a/Library/Homebrew/test/cache_store_spec.rb b/Library/Homebrew/test/cache_store_spec.rb index b2fecff153..e4363baf11 100644 --- a/Library/Homebrew/test/cache_store_spec.rb +++ b/Library/Homebrew/test/cache_store_spec.rb @@ -118,15 +118,15 @@ describe CacheStoreDatabase do end describe "#created?" do - let(:cache_path) { "path/to/homebrew/cache/sample.db" } + let(:cache_path) { Pathname("path/to/homebrew/cache/sample.db") } before(:each) do allow(subject).to receive(:cache_path).and_return(cache_path) end - context "`File.exist?(cache_path)` returns `true`" do + context "`cache_path.exist?` returns `true`" do before(:each) do - allow(File).to receive(:exist?).with(cache_path).and_return(true) + allow(cache_path).to receive(:exist?).and_return(true) end it "returns `true`" do @@ -134,9 +134,9 @@ describe CacheStoreDatabase do end end - context "`File.exist?(cache_path)` returns `false`" do + context "`cache_path.exist?` returns `false`" do before(:each) do - allow(File).to receive(:exist?).with(cache_path).and_return(false) + allow(cache_path).to receive(:exist?).and_return(false) end it "returns `false`" do diff --git a/Library/Homebrew/utils/ruby.sh b/Library/Homebrew/utils/ruby.sh index 9711b3fbe4..6bee43498e 100644 --- a/Library/Homebrew/utils/ruby.sh +++ b/Library/Homebrew/utils/ruby.sh @@ -47,7 +47,7 @@ setup-ruby-path() { then odie "Failed to install vendor Ruby." fi - rm -rf "$vendor_dir/bundle/ruby" "$HOMEBREW_CACHE/linkage.db" + rm -rf "$vendor_dir/bundle/ruby" HOMEBREW_RUBY_PATH="$vendor_ruby_path" fi fi