replcompletions: Try to make the test more robust (#59166)

This is an alternative to #59161, attempting to fix the same observed CI
behavior. I don't think #59161 is the best way to fix it, as the point
of these tests is to make sure that REPL completions looks up the PATH
internally. Calling the path update function explicitly defeats that
somewhat. The extra synchronization here to make this deterministic is
messy, but I do think it makes the test closer to real-world usage.

The core attempted fix here is to move the read of the PATH_ locals
inside `maybe_spawn_cache_PATH` into the locked region. If they are not
under the lock, they could be unconditionally overwritten by a second
call to this function, causing issues in the state machine. I do not
know whether this is the cause of the observed CI hangs, but it's worth
fixing anyway.

Author: Keno

stdlib/REPL/src/REPLCompletions.jl

@@ -335,21 +335,23 @@ PATH_cache_condition::Union{Threads.Condition, Nothing} = nothing # used for syn
 next_cache_update::Float64 = 0.0
 function maybe_spawn_cache_PATH()
     global PATH_cache_task, PATH_cache_condition, next_cache_update
-    # Extract to local variables to enable flow-sensitive type inference for these global variables
-    PATH_cache_task_local = PATH_cache_task
-    PATH_cache_condition_local = PATH_cache_condition
     @lock PATH_cache_lock begin
+        # Extract to local variables to enable flow-sensitive type inference for these global variables
+        PATH_cache_task_local = PATH_cache_task
         PATH_cache_task_local isa Task && !istaskdone(PATH_cache_task_local) && return
         time() < next_cache_update && return
-        PATH_cache_task = Threads.@spawn begin
-            REPLCompletions.cache_PATH()
-            @lock PATH_cache_lock begin
-                next_cache_update = time() + 10 # earliest next update can run is 10s after
-                PATH_cache_task = nothing # release memory when done
-                PATH_cache_condition_local !== nothing && notify(PATH_cache_condition_local)
+        PATH_cache_task = PATH_cache_task_local = Threads.@spawn begin
+            try
+                REPLCompletions.cache_PATH()
+            finally
+                @lock PATH_cache_lock begin
+                    next_cache_update = time() + 10 # earliest next update can run is 10s after
+                    PATH_cache_task = nothing # release memory when done
+                    PATH_cache_condition_local = PATH_cache_condition
+                    PATH_cache_condition_local !== nothing && notify(PATH_cache_condition_local)
+                end
             end
         end
-        PATH_cache_task_local = PATH_cache_task
         Base.errormonitor(PATH_cache_task_local)
     end
 end

stdlib/REPL/test/replcompletions.jl

@@ -1095,7 +1095,7 @@ function test_only_arm_cache_refresh()
         # force the next cache update to happen immediately
         REPL.REPLCompletions.next_cache_update = 0
     end
-    return REPL.REPLCompletions.PATH_cache_condition
+    return nothing
 end
 
 function test_only_wait_cache_path_done()