Skip to content

Commit

Permalink
Fix memory corruption if task is launched inside finalizer (#50597)
Browse files Browse the repository at this point in the history
In #48919, the tid selection logic inside `enq_task` gained a
`!GC.in_finalizer()` condition. However, this made it possible for
`workqueue_at` to be reached with `tid==0`, which would attempt and
out-of-bounds write under `@inbounds`, corrupting memory. This was not
caught in the test suite despite `--check-bounds=yes`, because our
`--check-bounds=yes` is currently best effort. That would be fixed by
#50239, which exposed this bug. This PR attempts to
fix this by marking any tasks launched inside a finalizer as not sticky.
Finalizers don't have any thread they run on
semantically, so i don't think there's a meaningful sense in which tasks
launched inside finalizers could be sticky.

(cherry picked from commit bd8350b)
  • Loading branch information
Keno authored and KristofferC committed Jul 24, 2023
1 parent a5d78d0 commit a2fda25
Showing 1 changed file with 12 additions and 4 deletions.
16 changes: 12 additions & 4 deletions base/task.jl
Original file line number Diff line number Diff line change
Expand Up @@ -774,17 +774,25 @@ function enq_work(t::Task)
# Sticky tasks go into their thread's work queue.
if t.sticky
tid = Threads.threadid(t)
if tid == 0 && !GC.in_finalizer()
if tid == 0
# The task is not yet stuck to a thread. Stick it to the current
# thread and do the same to the parent task (the current task) so
# that the tasks are correctly co-scheduled (issue #41324).
# XXX: Ideally we would be able to unset this.
tid = Threads.threadid()
ccall(:jl_set_task_tid, Cint, (Any, Cint), t, tid-1)
current_task().sticky = true
if GC.in_finalizer()
# The task was launched in a finalizer. There is no thread to sticky it
# to, so just allow it to run anywhere as if it had been non-sticky.
t.sticky = false
@goto not_sticky
else
tid = Threads.threadid()
ccall(:jl_set_task_tid, Cint, (Any, Cint), t, tid-1)
current_task().sticky = true
end
end
push!(workqueue_for(tid), t)
else
@label not_sticky
tp = Threads.threadpool(t)
if Threads.threadpoolsize(tp) == 1
# There's only one thread in the task's assigned thread pool;
Expand Down

0 comments on commit a2fda25

Please sign in to comment.