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 task switch in GC finalizer #4

Merged
merged 3 commits into from
Mar 5, 2024

Conversation

Octogonapus
Copy link
Contributor

@Octogonapus Octogonapus commented Mar 5, 2024

I encountered this error:

error in running finalizer: ErrorException("task switch not allowed from inside gc finalizer")
ijl_error at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/rtutils.c:41
ijl_switch at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/task.c:636
try_yieldto at ./task.jl:921
wait at ./task.jl:995
#wait#645 at ./condition.jl:130
wait at ./condition.jl:125 [inlined]
close at ./asyncevent.jl:180
#4 at /home/runner/.julia/packages/ForeignCallbacks/fneAK/src/ForeignCallbacks.jl:117
unknown function (ip: 0x7f2a34baa335)
_jl_invoke at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/gf.c:2894 [inlined]
ijl_apply_generic at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/gf.c:3076
run_finalizer at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/gc.c:318
jl_gc_run_finalizers_in_list at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/gc.c:408
run_finalizers at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/gc.c:454
jl_mutex_unlock at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/julia_locks.h:80 [inlined]
jl_generate_fptr_impl at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/jitlayers.cpp:525
jl_compile_method_internal at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/gf.c:2480 [inlined]
jl_compile_method_internal at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/gf.c:2368
_jl_invoke at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/gf.c:2886 [inlined]
ijl_apply_generic at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/gf.c:3076
_fire_post_update_callback at /home/runner/.julia/packages/AWSCRT/P0HMS/src/ShadowFramework.jl:352
_do_local_shadow_update! at /home/runner/.julia/packages/AWSCRT/P0HMS/src/ShadowFramework.jl:337
unknown function (ip: 0x7f2a34ba22d9)
_jl_invoke at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/gf.c:2894 [inlined]
ijl_apply_generic at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/gf.c:3076
_update_local_shadow_from_delta! at /home/runner/.julia/packages/AWSCRT/P0HMS/src/ShadowFramework.jl:309
shadow_callback at /home/runner/.julia/packages/AWSCRT/P0HMS/src/ShadowFramework.jl:256
#71 at /home/runner/.julia/packages/AWSCRT/P0HMS/src/IOTShadow.jl:74 [inlined]
#51 at /home/runner/.julia/packages/AWSCRT/P0HMS/src/AWSMQTT.jl:595
#3 at /home/runner/.julia/packages/ForeignCallbacks/fneAK/src/ForeignCallbacks.jl:110
unknown function (ip: 0x7f2a34b9d8d2)
_jl_invoke at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/gf.c:2894 [inlined]
ijl_apply_generic at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/gf.c:3076
jl_apply at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/julia.h:1982 [inlined]
start_task at /cache/build/builder-amdci4-6/julialang/julia-release-1-dot-10/src/task.c:1238

It looks like this task switch is caused by the close call in the finalizer here.
This makes sense to me; close is IO after all.

My solution is to wrap close in a new task to defer any potential task switch.

@Octogonapus Octogonapus force-pushed the fix_task_switch_in_gc branch from 2dcc548 to b87a4dd Compare March 5, 2024 15:30
@Octogonapus Octogonapus force-pushed the fix_task_switch_in_gc branch from b87a4dd to 50078b6 Compare March 5, 2024 15:32
src/ForeignCallbacks.jl Outdated Show resolved Hide resolved
@vchuravy
Copy link
Owner

vchuravy commented Mar 5, 2024

Thanks for the PR. I am not currently planning on maintaining this package since it is no longer useful (since Julia 1.9), but I will happily take your PR and release a version.

@Octogonapus
Copy link
Contributor Author

since it is no longer useful (since Julia 1.9)

Really? How else could I implement something like this: https://github.com/Octogonapus/AWSCRT.jl/blob/main/src/AWSMQTT.jl#L185-L190

Thanks for taking this PR though

@vchuravy
Copy link
Owner

vchuravy commented Mar 5, 2024

On Julia 1.9+ (JuliaLang/julia#46609) you can execute full Julia functions on foreign threads. So you could use a Channel or any other concurrent-safe datastructure.

@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment.

Thanks for integrating Codecov - We've got you covered ☂️

@vchuravy vchuravy merged commit 2815cc6 into vchuravy:main Mar 5, 2024
6 checks passed
@Octogonapus Octogonapus deleted the fix_task_switch_in_gc branch March 5, 2024 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants