diff --git a/CHANGELOG.md b/CHANGELOG.md index 174d7d4..7091fbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +* Fix the cache corruption issue in the revalidation feature. See #474. + # 1.18.2 * Disable stale cache entries revalidation by default as it seems to cause cache corruption issues. See #471 and #474. diff --git a/ext/bootsnap/bootsnap.c b/ext/bootsnap/bootsnap.c index db99408..e97919a 100644 --- a/ext/bootsnap/bootsnap.c +++ b/ext/bootsnap/bootsnap.c @@ -124,7 +124,7 @@ static void bs_cache_path(const char * cachedir, const VALUE path, char (* cache static int bs_read_key(int fd, struct bs_cache_key * key); static enum cache_status cache_key_equal_fast_path(struct bs_cache_key * k1, struct bs_cache_key * k2); static int cache_key_equal_slow_path(struct bs_cache_key * current_key, struct bs_cache_key * cached_key, const VALUE input_data); -static int update_cache_key(struct bs_cache_key *current_key, int cache_fd, const char ** errno_provenance); +static int update_cache_key(struct bs_cache_key *current_key, struct bs_cache_key *old_key, int cache_fd, const char ** errno_provenance); static void bs_cache_key_digest(struct bs_cache_key * key, const VALUE input_data); static VALUE bs_fetch(char * path, VALUE path_v, char * cache_path, VALUE handler, VALUE args); @@ -353,10 +353,11 @@ static int cache_key_equal_slow_path(struct bs_cache_key *current_key, return current_key->digest == cached_key->digest; } -static int update_cache_key(struct bs_cache_key *current_key, int cache_fd, const char ** errno_provenance) +static int update_cache_key(struct bs_cache_key *current_key, struct bs_cache_key *old_key, int cache_fd, const char ** errno_provenance) { + old_key->mtime = current_key->mtime; lseek(cache_fd, 0, SEEK_SET); - ssize_t nwrite = write(cache_fd, current_key, KEY_SIZE); + ssize_t nwrite = write(cache_fd, old_key, KEY_SIZE); if (nwrite < 0) { *errno_provenance = "update_cache_key:write"; return -1; @@ -825,7 +826,7 @@ bs_fetch(char * path, VALUE path_v, char * cache_path, VALUE handler, VALUE args valid_cache = cache_key_equal_slow_path(¤t_key, &cached_key, input_data); if (valid_cache) { if (!readonly) { - if (update_cache_key(¤t_key, cache_fd, &errno_provenance)) { + if (update_cache_key(¤t_key, &cached_key, cache_fd, &errno_provenance)) { exception_message = path_v; goto fail_errno; } @@ -993,7 +994,7 @@ bs_precompile(char * path, VALUE path_v, char * cache_path, VALUE handler) } valid_cache = cache_key_equal_slow_path(¤t_key, &cached_key, input_data); if (valid_cache) { - if (update_cache_key(¤t_key, cache_fd, &errno_provenance)) { + if (update_cache_key(¤t_key, &cached_key, cache_fd, &errno_provenance)) { goto fail; } } diff --git a/test/compile_cache_key_format_test.rb b/test/compile_cache_key_format_test.rb index 5108374..c4286d1 100644 --- a/test/compile_cache_key_format_test.rb +++ b/test/compile_cache_key_format_test.rb @@ -20,6 +20,11 @@ class CompileCacheKeyFormatTest < Minitest::Test data_size: 32...40, }.freeze + def teardown + Bootsnap::CompileCache::Native.revalidation = false + super + end + def test_key_version key = cache_key_for_file(FILE) exp = [5].pack("L") @@ -79,6 +84,22 @@ def test_fetch assert_equal("NEATO #{target.upcase}", actual) end + def test_revalidation + Bootsnap::CompileCache::Native.revalidation = true + cache_dir = File.join(@tmp_dir, "compile_cache") + + target = Help.set_file("a.rb", "foo = 1") + + actual = Bootsnap::CompileCache::Native.fetch(cache_dir, target, TestHandler, nil) + assert_equal("NEATO #{target.upcase}", actual) + + 10.times do + FileUtils.touch(target, mtime: File.mtime(target) + 42) + actual = Bootsnap::CompileCache::Native.fetch(cache_dir, target, TestHandler, nil) + assert_equal("NEATO #{target.upcase}", actual) + end + end + def test_unexistent_fetch assert_raises(Errno::ENOENT) do Bootsnap::CompileCache::Native.fetch(@tmp_dir, "123", Bootsnap::CompileCache::ISeq, nil) diff --git a/test/compile_cache_test.rb b/test/compile_cache_test.rb index 62dea08..da83dec 100644 --- a/test/compile_cache_test.rb +++ b/test/compile_cache_test.rb @@ -160,6 +160,24 @@ def test_dont_store_cache_after_a_stale_when_readonly load(path) end + def test_revalidation + Bootsnap::CompileCache::Native.revalidation = true + + file_path = Help.set_file("a.rb", "a = a = 3", 100) + load(file_path) + + calls = [] + Bootsnap.instrumentation = ->(event, path) { calls << [event, path] } + + 5.times do + FileUtils.touch("a.rb", mtime: File.mtime("a.rb") + 42) + load(file_path) + load(file_path) + end + + assert_equal [[:revalidated, "a.rb"], [:hit, "a.rb"]] * 5, calls + end + def test_dont_revalidate_when_readonly Bootsnap::CompileCache::Native.revalidation = true