Skip to content

Commit

Permalink
fix #32970, at-threads disabled after a loop errors (#33034)
Browse files Browse the repository at this point in the history
  • Loading branch information
JeffBezanson committed Aug 26, 2019
1 parent e6dd72f commit 5c42f10
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 9 deletions.
11 changes: 2 additions & 9 deletions base/threadingconstructs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ on `threadid()`.
"""
nthreads() = Int(unsafe_load(cglobal(:jl_n_threads, Cint)))

# Only read/written by the main thread
const in_threaded_loop = Ref(false)

function _threadsfor(iter,lbody)
lidx = iter.args[1] # index
range = iter.args[2]
Expand Down Expand Up @@ -65,15 +62,11 @@ function _threadsfor(iter,lbody)
end
end
end
# Hack to make nested threaded loops kinda work
if threadid() != 1 || in_threaded_loop[]
# We are in a nested threaded loop
if threadid() != 1
# only thread 1 can enter/exit _threadedregion
Base.invokelatest(threadsfor_fun, true)
else
in_threaded_loop[] = true
# the ccall is not expected to throw
ccall(:jl_threading_run, Cvoid, (Any,), threadsfor_fun)
in_threaded_loop[] = false
end
nothing
end
Expand Down
16 changes: 16 additions & 0 deletions test/threads_exec.jl
Original file line number Diff line number Diff line change
Expand Up @@ -670,3 +670,19 @@ let ch = Channel{Char}(0), t
schedule(t)
@test String(collect(ch)) == "hello"
end

# errors inside @threads
function _atthreads_with_error(a, err)
Threads.@threads for i in eachindex(a)
if err
error("failed")
end
a[i] = Threads.threadid()
end
a
end
@test_throws TaskFailedException _atthreads_with_error(zeros(nthreads()), true)
let a = zeros(nthreads())
_atthreads_with_error(a, false)
@test a == [1:nthreads();]
end

2 comments on commit 5c42f10

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

Please sign in to comment.