Skip to content

Commit

Permalink
Fix memory corruption if task is launched inside finalizer
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.
  • Loading branch information
Keno committed Jul 19, 2023
1 parent 2cee483 commit 2899656
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 @@ -776,17 +776,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 2899656

Please sign in to comment.