Merge pull request #475 from Shopify/disable-revalidation-by-default

Disable stale cache entries revalidation by default

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Unreleased
 
+* Disable stale cache entries revalidation by default as it seems to cause cache corruption issues. See #471 and #474.
+  Will be re-enabled in a future version once the root cause is identified.
+
 * Fix a potential compilation issue on some systems. See #470.
 
 # 1.18.1

ext/bootsnap/bootsnap.c

@@ -103,11 +103,13 @@ static ID instrumentation_method;
 static VALUE sym_hit, sym_miss, sym_stale, sym_revalidated;
 static bool instrumentation_enabled = false;
 static bool readonly = false;
+static bool revalidation = false;
 static bool perm_issue = false;
 
 /* Functions exposed as module functions on Bootsnap::CompileCache::Native */
 static VALUE bs_instrumentation_enabled_set(VALUE self, VALUE enabled);
 static VALUE bs_readonly_set(VALUE self, VALUE enabled);
+static VALUE bs_revalidation_set(VALUE self, VALUE enabled);
 static VALUE bs_compile_option_crc32_set(VALUE self, VALUE crc32_v);
 static VALUE bs_rb_fetch(VALUE self, VALUE cachedir_v, VALUE path_v, VALUE handler, VALUE args);
 static VALUE bs_rb_precompile(VALUE self, VALUE cachedir_v, VALUE path_v, VALUE handler);
@@ -190,6 +192,7 @@ Init_bootsnap(void)
 
   rb_define_module_function(rb_mBootsnap, "instrumentation_enabled=", bs_instrumentation_enabled_set, 1);
   rb_define_module_function(rb_mBootsnap_CompileCache_Native, "readonly=", bs_readonly_set, 1);
+  rb_define_module_function(rb_mBootsnap_CompileCache_Native, "revalidation=", bs_revalidation_set, 1);
   rb_define_module_function(rb_mBootsnap_CompileCache_Native, "coverage_running?", bs_rb_coverage_running, 0);
   rb_define_module_function(rb_mBootsnap_CompileCache_Native, "fetch", bs_rb_fetch, 4);
   rb_define_module_function(rb_mBootsnap_CompileCache_Native, "precompile", bs_rb_precompile, 3);
@@ -221,6 +224,13 @@ bs_readonly_set(VALUE self, VALUE enabled)
   return enabled;
 }
 
+static VALUE
+bs_revalidation_set(VALUE self, VALUE enabled)
+{
+  revalidation = RTEST(enabled);
+  return enabled;
+}
+
 /*
  * Bootsnap's ruby code registers a hook that notifies us via this function
  * when compile_option changes. These changes invalidate all existing caches.
@@ -325,7 +335,12 @@ static enum cache_status cache_key_equal_fast_path(struct bs_cache_key *k1,
           k1->ruby_platform == k2->ruby_platform &&
           k1->compile_option == k2->compile_option &&
           k1->ruby_revision == k2->ruby_revision && k1->size == k2->size) {
-    return (k1->mtime == k2->mtime) ? hit : stale;
+      if (k1->mtime == k2->mtime) {
+        return hit;
+      }
+      if (revalidation) {
+        return stale;
+      }
   }
   return miss;
 }
@@ -515,7 +530,7 @@ open_cache_file(const char * path, struct bs_cache_key * key, const char ** errn
 {
   int fd, res;
 
-  if (readonly) {
+  if (readonly || !revalidation) {
     fd = bs_open_noatime(path, O_RDONLY);
   } else {
     fd = bs_open_noatime(path, O_RDWR);

lib/bootsnap.rb

@@ -51,6 +51,7 @@ def setup(
       load_path_cache: true,
       ignore_directories: nil,
       readonly: false,
+      revalidation: false,
       compile_cache_iseq: true,
       compile_cache_yaml: true,
       compile_cache_json: true
@@ -70,6 +71,7 @@ def setup(
         yaml: compile_cache_yaml,
         json: compile_cache_json,
         readonly: readonly,
+        revalidation: revalidation,
       )
     end
 

lib/bootsnap/compile_cache.rb

@@ -9,7 +9,7 @@ def UNCOMPILABLE.inspect
 
     Error = Class.new(StandardError)
 
-    def self.setup(cache_dir:, iseq:, yaml:, json:, readonly: false)
+    def self.setup(cache_dir:, iseq:, yaml:, json:, readonly: false, revalidation: false)
       if iseq
         if supported?
           require_relative "compile_cache/iseq"
@@ -39,6 +39,7 @@ def self.setup(cache_dir:, iseq:, yaml:, json:, readonly: false)
 
       if supported? && defined?(Bootsnap::CompileCache::Native)
         Bootsnap::CompileCache::Native.readonly = readonly
+        Bootsnap::CompileCache::Native.revalidation = revalidation
       end
     end
 

test/compile_cache_test.rb

@@ -9,6 +9,7 @@ class CompileCacheTest < Minitest::Test
   def teardown
     super
     Bootsnap::CompileCache::Native.readonly = false
+    Bootsnap::CompileCache::Native.revalidation = false
     Bootsnap.instrumentation = nil
   end
 
@@ -160,6 +161,8 @@ def test_dont_store_cache_after_a_stale_when_readonly
   end
 
   def test_dont_revalidate_when_readonly
+    Bootsnap::CompileCache::Native.revalidation = true
+
     path = Help.set_file("a.rb", "a = a = 3", 100)
     load(path)
 
@@ -220,6 +223,8 @@ def test_instrumentation_miss
   end
 
   def test_instrumentation_revalidate
+    Bootsnap::CompileCache::Native.revalidation = true
+
     file_path = Help.set_file("a.rb", "a = a = 3", 100)
     load(file_path)
     FileUtils.touch("a.rb", mtime: File.mtime("a.rb") + 42)