Skip to content

Commit 50e72b3

Browse files
KenoKristofferC
authored andcommitted
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. (cherry picked from commit 9a16119)
1 parent 552d97c commit 50e72b3

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
342341
PATH_cache_task_local isa Task && !istaskdone(PATH_cache_task_local) && return
343342
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)
343+
PATH_cache_task = PATH_cache_task_local = Threads.@spawn begin
344+
try
345+
REPLCompletions.cache_PATH()
346+
finally
347+
@lock PATH_cache_lock begin
348+
next_cache_update = time() + 10 # earliest next update can run is 10s after
349+
PATH_cache_task = nothing # release memory when done
350+
PATH_cache_condition_local = PATH_cache_condition
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
@@ -1092,7 +1092,7 @@ function test_only_arm_cache_refresh()
10921092
# force the next cache update to happen immediately
10931093
REPL.REPLCompletions.next_cache_update = 0
10941094
end
1095-
return REPL.REPLCompletions.PATH_cache_condition
1095+
return nothing
10961096
end
10971097

10981098
function test_only_wait_cache_path_done()

0 commit comments

Comments
 (0)