Make threadcall gc safe #55956
Make threadcall gc safe #55956
Conversation
gbaraldi
added
backport 1.10
|
Someone should finish #49933 |
|
Just to confirm, is it safe to call |
|
"GC safepoint" are singular points in time. GC safe regions are spans in time, and yes it is legal to to nest safe regions. |
base/threadcall.jl
Outdated
| @@ -43,7 +43,9 @@ macro threadcall(f, rettype, argtypes, argvals...) | |||
| push!(body, :(p += Core.sizeof($T))) | |||
| push!(args, arg) | |||
| end | |||
| push!(body, :(gc_state = ccall(:jl_gc_safe_enter, Int8, (), ))) | |||
There was a problem hiding this comment.
So a problem with this is that the ccall lowering will insert calls to cconvert and then unsafe_convert. cconvert is designed to allocate.
So you manually need to insert the code to lower include aGC.@preserve
There was a problem hiding this comment.
There was a problem hiding this comment.
Have I told you how much I dislike threadcall?
There was a problem hiding this comment.
Also see JuliaInterop/ObjectiveC.jl@2578dc2, which is even more relevant (it turns a ccall into a GC-safe ccall after manually doing argument conversion first). It would almost be easier to fix #49933 at this point...
There was a problem hiding this comment.
The issue is that we need to backport this :(
gbaraldi
force-pushed
the
gb/threadcall-gc
branch
from
73739a1 to
d4108ef
Compare
| @@ -39,13 +39,19 @@ macro threadcall(f, rettype, argtypes, argvals...) | |||
| args = Symbol[] | |||
| for (i, T) in enumerate(argtypes) | |||
| arg = Symbol("arg", i) | |||
| push!(body, :($arg = unsafe_load(convert(Ptr{$T}, p)))) | |||
| push!(body, :($arg = unsafe_convert($T, cconvert($T, unsafe_load(convert(Ptr{$T}, p)))))) | |||
There was a problem hiding this comment.
I don't fully understand the code here (why is there an unsafe_load), but the return from cconvert must be GC preserved.
There was a problem hiding this comment.
Threadcall passes the args in an array of ptrs if I understood the code correctly
| push!(body, :(unsafe_store!(convert(Ptr{$rettype}, retval_ptr), ret))) | ||
| push!(body, :(return Int(Core.sizeof($rettype)))) | ||
| append!(body, (quote | ||
| GC.@preserve $(args...) begin |
There was a problem hiding this comment.
You are preserving the unsafe_convert not the cconvert here, right?
threadcall cannot call julia code. But due to thread adoption it currently is not marked gc safe. Which means that a blocking call will cause a hang in the GC