Skip to content

Commit 1611730

Browse files
committed
Fix update_status race by snapshotting under the lock instead.
1 parent 3d77459 commit 1611730

1 file changed

Lines changed: 16 additions & 10 deletions

File tree

src/ParallelTestRunner.jl

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,16 +1003,19 @@ function runtests(mod::Module, args::ParsedArgs;
10031003
end
10041004

10051005
function update_status()
1006-
# only draw if we have something to show
1007-
@lock(running_tests, isempty(running_tests[])) && return
1008-
completed = @lock results length(results[])
1006+
# take consistent snapshots once, so the rest of this function operates on
1007+
# frozen data rather than racing with workers that mutate these collections
1008+
running_snapshot = @lock running_tests copy(running_tests[])
1009+
isempty(running_snapshot) && return
1010+
results_snapshot = @lock results copy(results[])
1011+
completed = length(results_snapshot)
10091012
total = @lock tests length(tests[])
10101013

10111014
# line 1: empty line
10121015
line1 = ""
10131016

10141017
# line 2: running tests
1015-
test_list = @lock running_tests sort(collect(keys(running_tests[])), by = x -> running_tests[][x])
1018+
test_list = sort(collect(keys(running_snapshot)), by = x -> running_snapshot[x])
10161019
status_parts = map(test_list) do test
10171020
"$test"
10181021
end
@@ -1027,23 +1030,23 @@ function runtests(mod::Module, args::ParsedArgs;
10271030
line3 = "Progress: $completed/$total tests completed"
10281031
if completed > 0
10291032
# estimate per-test time (slightly pessimistic)
1030-
durations_done = @lock results [end_time - start_time for (_, _,_, start_time, end_time) in results[]]
1033+
durations_done = [end_time - start_time for (_, _,_, start_time, end_time) in results_snapshot]
10311034
μ = mean(durations_done)
10321035
σ = length(durations_done) > 1 ? std(durations_done) : 0.0
10331036
est_per_test = μ + 0.5σ
10341037

10351038
est_remaining = 0.0
10361039
## currently-running
1037-
for (test, start_time) in @lock(running_tests, running_tests[])
1040+
for (test, start_time) in running_snapshot
10381041
elapsed = time() - start_time
10391042
duration = get(historical_durations, test, est_per_test)
10401043
est_remaining += max(0.0, duration - elapsed)
10411044
end
10421045
## yet-to-run
1043-
for test in @lock(tests, tests[])
1044-
@lock(running_tests, haskey(running_tests[], test)) && continue
1046+
@lock tests for test in tests[]
1047+
haskey(running_snapshot, test) && continue
10451048
# Test is in any completed test
1046-
@lock(results, any(r -> test == r.test, results[])) && continue
1049+
any(r -> test == r.test, results_snapshot) && continue
10471050
est_remaining += get(historical_durations, test, est_per_test)
10481051
end
10491052

@@ -1126,7 +1129,10 @@ function runtests(mod::Module, args::ParsedArgs;
11261129
end
11271130
isa(ex, InterruptException) || rethrow()
11281131
finally
1129-
if @lock running_tests @lock results @lock tests isempty(running_tests[]) && length(results[]) >= length(tests[])
1132+
n_running = @lock running_tests length(running_tests[])
1133+
n_results = @lock results length(results[])
1134+
n_tests = @lock tests length(tests[])
1135+
if n_running == 0 && n_results >= n_tests
11301136
# XXX: only erase the status if we completed successfully.
11311137
# in other cases we'll have printed "caught interrupt"
11321138
clear_status()

0 commit comments

Comments
 (0)