Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix segfault while profiling multitasking #42973

Merged
merged 3 commits into from
Nov 8, 2021

Conversation

tkf
Copy link
Member

@tkf tkf commented Nov 6, 2021

In 1.8-DEV (2ece1cd), the following script segfaults

using Profile

function spawnmany(n)
    if n > 2
        m = n ÷ 2
        t = Threads.@spawn spawnmany(m)
        spawnmany(m)
        wait(t)
    end
end

@time @profile spawnmany(2000000)

This seems to occur essentially because SIGUSR2 handler can observe jl_get_current_task()->ptls == NULL. This patch simply adds a null check in the handler.

I think the segfault is not unexpected since lastt->ptls = NULL and jl_set_pgcstack(&t->gcstack) doesn't happen atomically in the task switch:

julia/src/task.c

Lines 434 to 438 in 2ece1cd

lastt->ptls = NULL;
#ifdef MIGRATE_TASKS
ptls->previous_task = lastt;
#endif
jl_set_pgcstack(&t->gcstack);

The above script still doesn't work in 1.8-DEV due to (presumably) some deadlock issue. But I think this patch can already be applied.

I can reproduce this in 1.7.0-rc2.31 (75961b2) so this needs to be backported. (Interestingly, I couldn't get the same issue in 1.7.0-rc1.63 e888fb8; So the bug may be introduced or surfaced between this window.)

@tkf tkf requested a review from vtjnash November 6, 2021 20:10
@tkf tkf added backport 1.7 kind:bugfix This change fixes an existing bug labels Nov 6, 2021
@tkf tkf added the domain:multithreading Base.Threads and related functionality label Nov 6, 2021
@vtjnash vtjnash merged commit 12b9bec into JuliaLang:master Nov 8, 2021
@tkf tkf deleted the profile-segfault branch November 8, 2021 18:47
KristofferC pushed a commit that referenced this pull request Nov 10, 2021
@tkf tkf added the profiler label Nov 14, 2021
daviehh pushed a commit to daviehh/julia that referenced this pull request Nov 16, 2021
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:multithreading Base.Threads and related functionality kind:bugfix This change fixes an existing bug profiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants