-
Notifications
You must be signed in to change notification settings - Fork 3
Description
Hello,
From the following PkgEval log: https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/5da257d_vs_d63aded/FractalDimensions.primary.log it seems likely that this package is using Threads.nthreads
and Threads.threadid
incorrectly as described in https://julialang.org/blog/2023/07/PSA-dont-use-threadid/ and https://juliafolds2.github.io/OhMyThreads.jl/stable/literate/tls/tls/#The-naive-(and-incorrect)-approach.
In the upcoming Julia 1.12, julia runs with an interactive thread by default which means that the number of worker threads nthreads()
defaults to 1 but the thread id that things will run on in a threaded context will typically be 2 (the interactive thread having the id 1):
julia> Threads.nthreads()
1
julia> Threads.@threads for i = 1:3
@show Threads.threadid()
end
Threads.threadid() = 2
Threads.threadid() = 2
Threads.threadid() = 2
Trying to index a vector of length Threads.nthreads()
with Threads.threadid()
will thus error.
Note that even though this code only started directly erroring in 1.12 the code was likely already incorrect on earlier Julia versions unless it made sure that the spawned tasks did not migrate between threads (which can happen at any yield point).
Some remedies to this are:
- Use the new
OncePerThread
(orOncePerTask
) functionality added in 1.12. - Use task local values instead of thread local values as described in https://juliafolds2.github.io/OhMyThreads.jl/stable/literate/tls/tls/#TLV
- Use a
Channel
as described in https://juliafolds2.github.io/OhMyThreads.jl/stable/literate/tls/tls/#The-safe-way:-Channel. - Use
Threads.maxthreadid()
instead ofThreads.nthreads()
. This can waste some memory sincemaxthreadid()
can give a higher number than the number of worker threads. It also doesn't help against the task migration issue described earlier (but the previous code did not do that either).