Skip to content

Commit 07c51a8

Browse files
committed
replcompletions: Try to make the test more robust
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.
1 parent c9ef3a1 commit 07c51a8

File tree

2 files changed

+13
-11
lines changed

2 files changed

+13
-11
lines changed

stdlib/REPL/src/REPLCompletions.jl

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -335,21 +335,23 @@ PATH_cache_condition::Union{Threads.Condition, Nothing} = nothing # used for syn
335335
next_cache_update::Float64 = 0.0
336336
function maybe_spawn_cache_PATH()
337337
global PATH_cache_task, PATH_cache_condition, next_cache_update
338-
# Extract to local variables to enable flow-sensitive type inference for these global variables
339-
PATH_cache_task_local = PATH_cache_task
340-
PATH_cache_condition_local = PATH_cache_condition
341338
@lock PATH_cache_lock begin
339+
# Extract to local variables to enable flow-sensitive type inference for these global variables
340+
PATH_cache_task_local = PATH_cache_task
341+
PATH_cache_condition_local = PATH_cache_condition
342342
PATH_cache_task_local isa Task && !istaskdone(PATH_cache_task_local) && return
343343
time() < next_cache_update && return
344-
PATH_cache_task = Threads.@spawn begin
345-
REPLCompletions.cache_PATH()
346-
@lock PATH_cache_lock begin
347-
next_cache_update = time() + 10 # earliest next update can run is 10s after
348-
PATH_cache_task = nothing # release memory when done
349-
PATH_cache_condition_local !== nothing && notify(PATH_cache_condition_local)
344+
PATH_cache_task = PATH_cache_task_local = Threads.@spawn begin
345+
try
346+
REPLCompletions.cache_PATH()
347+
finally
348+
@lock PATH_cache_lock begin
349+
next_cache_update = time() + 10 # earliest next update can run is 10s after
350+
PATH_cache_task = nothing # release memory when done
351+
PATH_cache_condition_local !== nothing && notify(PATH_cache_condition_local)
352+
end
350353
end
351354
end
352-
PATH_cache_task_local = PATH_cache_task
353355
Base.errormonitor(PATH_cache_task_local)
354356
end
355357
end

stdlib/REPL/test/replcompletions.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1095,7 +1095,7 @@ function test_only_arm_cache_refresh()
10951095
# force the next cache update to happen immediately
10961096
REPL.REPLCompletions.next_cache_update = 0
10971097
end
1098-
return REPL.REPLCompletions.PATH_cache_condition
1098+
return nothing
10991099
end
11001100

11011101
function test_only_wait_cache_path_done()

0 commit comments

Comments
 (0)