fix(runtime): make a copy of runtime_search_path when iterating

This is to prevent concurrent modification, just like save_rtp
in the vim 8 implementation

src/nvim/option.c

@@ -2398,7 +2398,7 @@ static char_u *did_set_string_option(int opt_idx, char_u **varp, bool new_value_
       didset_vimruntime = false;
     }
   } else if (varp == &p_rtp || varp == &p_pp) {  // 'runtimepath' 'packpath'
-    invalidate_search_path();
+    runtime_search_path_invalidate();
   } else if (varp == &curwin->w_p_culopt
              || gvarp == &curwin->w_allbuf_opt.wo_culopt) {  // 'cursorlineopt'
     if (**varp == NUL || fill_culopt_flags(*varp, curwin) != OK) {

src/nvim/runtime.c

@@ -21,7 +21,8 @@
 # include "runtime.c.generated.h"
 #endif
 
-static bool search_path_valid = false;
+static bool runtime_search_path_valid = false;
+static int *runtime_search_path_ref = NULL;
 static RuntimeSearchPath runtime_search_path;
 
 /// ":runtime [what] {name}"
@@ -167,7 +168,7 @@ int do_in_path(char_u *path, char_u *name, int flags, DoInRuntimepathCB callback
 /// return FAIL when no file could be sourced, OK otherwise.
 int do_in_cached_path(char_u *name, int flags, DoInRuntimepathCB callback, void *cookie)
 {
-  validate_search_path();
+  runtime_search_path_validate();
   char_u      *tail;
   int num_files;
   char_u      **files;
@@ -182,9 +183,18 @@ int do_in_cached_path(char_u *name, int flags, DoInRuntimepathCB callback, void
     verbose_leave();
   }
 
-  // Loop over all entries in 'runtimepath'.
-  for (size_t j = 0; j < kv_size(runtime_search_path); j++) {
-    SearchPathItem item = kv_A(runtime_search_path, j);
+  RuntimeSearchPath path = runtime_search_path;
+  int ref = 0;
+  if (runtime_search_path_ref == NULL) {
+    // cached path was unreferenced. keep a ref to
+    // prevent runtime_search_path() to freeing it too early
+    ref++;
+    runtime_search_path_ref = &ref;
+  }
+
+  // Loop over all entries in cached path
+  for (size_t j = 0; j < kv_size(path); j++) {
+    SearchPathItem item = kv_A(path, j);
     size_t buflen = strlen(item.path);
 
     // Skip after or non-after directories.
@@ -246,6 +256,14 @@ int do_in_cached_path(char_u *name, int flags, DoInRuntimepathCB callback, void
     }
   }
 
+  if (ref) {
+    if (runtime_search_path_ref == &ref) {
+      runtime_search_path_ref = NULL;
+    } else {
+      runtime_search_path_free(path);
+    }
+  }
+
 
   return did_one ? OK : FAIL;
 }
@@ -361,7 +379,7 @@ static bool path_is_after(char_u *buf, size_t buflen)
          && STRCMP(buf + buflen - 5, "after") == 0;
 }
 
-RuntimeSearchPath build_runtime_search_path(void)
+RuntimeSearchPath runtime_search_path_build(void)
 {
   kvec_t(String) pack_entries = KV_INITIAL_VALUE;
   Map(String, handle_T) pack_used = MAP_INIT;
@@ -426,21 +444,29 @@ RuntimeSearchPath build_runtime_search_path(void)
   return search_path;
 }
 
-void invalidate_search_path(void)
+void runtime_search_path_invalidate(void)
 {
-  search_path_valid = false;
+  runtime_search_path_valid = false;
 }
 
-void validate_search_path(void)
+void runtime_search_path_free(RuntimeSearchPath path)
 {
-  if (!search_path_valid) {
-    for (size_t j = 0; j < kv_size(runtime_search_path); j++) {
-      SearchPathItem item = kv_A(runtime_search_path, j);
+    for (size_t j = 0; j < kv_size(path); j++) {
+      SearchPathItem item = kv_A(path, j);
       xfree(item.path);
     }
-    kv_destroy(runtime_search_path);
-    runtime_search_path = build_runtime_search_path();
-    search_path_valid = true;
+    kv_destroy(path);
+}
+
+void runtime_search_path_validate(void)
+{
+  if (!runtime_search_path_valid) {
+    if (!runtime_search_path_ref) {
+      runtime_search_path_free(runtime_search_path);
+    }
+    runtime_search_path = runtime_search_path_build();
+    runtime_search_path_valid = true;
+    runtime_search_path_ref = NULL;  // initially unowned
   }
 }
 

test/functional/core/startup_spec.lua

@@ -358,6 +358,12 @@ describe('startup', function()
     pack_clear [[ lua _G.test_loadorder = {} vim.cmd "packadd! superspecial\nruntime! filen.lua" ]]
     eq({'ordinary', 'SuperSpecial', 'FANCY', 'FANCY after', 'SuperSpecial after', 'ordinary after'}, exec_lua [[ return _G.test_loadorder ]])
   end)
+
+  it("handles the correct order with a package that changes packpath", function()
+    pack_clear [[ lua _G.test_loadorder = {} vim.cmd "packadd! funky\nruntime! filen.lua" ]]
+    eq({'ordinary', 'funky!', 'FANCY', 'FANCY after', 'ordinary after'}, exec_lua [[ return _G.test_loadorder ]])
+    eq({'ordinary', 'funky!', 'ordinary after'}, exec_lua [[ return _G.nested_order ]])
+  end)
 end)
 
 describe('sysinit', function()

test/functional/fixtures/pack/foo/opt/funky/filen.lua

@@ -0,0 +1,12 @@
+table.insert(_G.test_loadorder, "funky!")
+
+if not _G.nesty then
+  _G.nesty = true
+  local save_order = _G.test_loadorder
+  _G.test_loadorder = {}
+  _G.vim.o.pp = "" -- funky!
+  vim.cmd [[runtime! filen.lua ]]
+  _G.nested_order = _G.test_loadorder
+  _G.test_loadorder = save_order
+  _G.nesty = nil
+end