Segfault due to MethodError when tracking Base #552
Comments
|
It's a good guess. Probably Revise deleted the method, but Revise itself depends on it, and some place where it's using runtime dispatch ended up getting stuck. This is probably not fixable unless we do something to make Revise un-invalidatable, kind of like Core.Compiler. @c42f's work on running in a particular world age might be a way to do that? |
|
I should also mention that this only happens for me if I add both |
|
Yes, sounds like you could try
I had a very quick look, but I think my efforts were undone by point (1) as it seems there's some tricky uses of |
|
The I'm guessing
I already do that (see |
|
An aside: since both of you have made changes to Revise, if you don't know already it's often useful to disable Line 186 in 10b6096 return bt to the top. It's there to prevent users from seeing long backtraces to Revise's own code, but when you're hacking on Revise you want to see them.
So, with these changes: diff --git a/src/legacy_loading.jl b/src/legacy_loading.jl
index e310f22..5a6448f 100644
--- a/src/legacy_loading.jl
+++ b/src/legacy_loading.jl
@@ -156,7 +156,7 @@ function parse_pkg_files(id::PkgId)
# To reduce compiler latency, use runtime dispatch for `queue_includes!`.
# `queue_includes!` requires compilation of the whole parsing/expression-splitting infrastructure,
# and it's better to wait to compile it until we actually need it.
- Base.invokelatest(queue_includes!, pkgdata, id)
+ Base.invoke_in_world(worldage[], queue_includes!, pkgdata, id)
return pkgdata
end
diff --git a/src/loading.jl b/src/loading.jl
index db8d0b2..0764985 100644
--- a/src/loading.jl
+++ b/src/loading.jl
@@ -48,7 +48,7 @@ function parse_pkg_files(id::PkgId)
# To reduce compiler latency, use runtime dispatch for `queue_includes!`.
# `queue_includes!` requires compilation of the whole parsing/expression-splitting infrastructure,
# and it's better to wait to compile it until we actually need it.
- Base.invokelatest(queue_includes!, pkgdata, id)
+ Base.invoke_in_world(worldage[], queue_includes!, pkgdata, id)
return pkgdata
end
diff --git a/src/packagedef.jl b/src/packagedef.jl
index 441c25a..c053b8a 100644
--- a/src/packagedef.jl
+++ b/src/packagedef.jl
@@ -198,6 +198,11 @@ const silence_pkgs = Set{Symbol}()
const depsdir = joinpath(dirname(@__DIR__), "deps")
const silencefile = Ref(joinpath(depsdir, "silence.txt")) # Ref so that tests don't clobber
+"""
+ world age
+"""
+const worldage = Ref{Union{Nothing,UInt}}(nothing)
+
##
## The inputs are sets of expressions found in each file.
## Some of those expressions will generate methods which are identified via their signatures.
@@ -1178,7 +1183,7 @@ end
# This uses invokelatest not for reasons of world age but to ensure that the call is made at runtime.
# This allows `revise_first` to be compiled without compiling `revise` itself, and greatly
# reduces the overhead of using Revise.
-revise_first(ex) = Expr(:toplevel, :(isempty($revision_queue) || Base.invokelatest($revise)), ex)
+revise_first(ex) = Expr(:toplevel, :(isempty($revision_queue) || Base.invoke_in_world($(worldage[]), $revise)), ex)
@noinline function run_backend(backend)
while true
@@ -1277,6 +1282,7 @@ function init_worker(p)
end
function __init__()
+ worldage[] = Base.get_world_counter()
run_on_worker = get(ENV, "JULIA_REVISE_WORKER_ONLY", "0")
if !(myid() == 1 || run_on_worker == "1")
return nothing
diff --git a/src/pkgs.jl b/src/pkgs.jl
index 95fbaa6..fb515f1 100644
--- a/src/pkgs.jl
+++ b/src/pkgs.jl
@@ -171,7 +171,7 @@ function maybe_add_includes_to_pkgdata!(pkgdata::PkgData, file::AbstractString,
parse_source!(fi.modexsigs, fullfile, mod)
if eval_now
# Use runtime dispatch to reduce latency
- Base.invokelatest(instantiate_sigs!, fi.modexsigs; mode=:eval)
+ Base.invoke_in_world(worldage[], instantiate_sigs!, fi.modexsigs; mode=:eval)
end
end
# Add to watchlist
@@ -237,7 +237,7 @@ function _add_require(sourcefile, modcaller, idmod, modname, expr)
end
end
if complex
- Base.invokelatest(eval_require_now, pkgdata, fileidx, filekey, sourcefile, modcaller, expr)
+ Base.invoke_in_world(worldage[], eval_require_now, pkgdata, fileidx, filekey, sourcefile, modcaller, expr)
end
finally
unlock(requires_lock)
diff --git a/src/utils.jl b/src/utils.jl
index 785c4ac..7b97483 100644
--- a/src/utils.jl
+++ b/src/utils.jl
@@ -184,6 +184,7 @@ println_maxsize(io::IO, args...; kwargs...) = printf_maxsize(println, io, args..
# Trimming backtraces
function trim_toplevel!(bt)
+ return bt
n = itoplevel = length(bt)
for (i, t) in enumerate(bt)
sfs = StackTraces.lookup(t)I then get So it seems the I've pushed my WIP as the |
|
@simeonschaub, can't resist asking: why would you need to force inlining on those range methods? That should happen automatically. |
|
Yes, you are absolutely right, it didn't end up doing anything. I was looking into the allocations in JuliaLang/julia#37888 and thought I'd just try that in case there was anything weird going on. The allocations came from somewhere else though. |
The problem seems to be that macro expansion always just uses the latest worldage: julia> macro m() 1 end
@m (macro with 1 method)
julia> wa = Base.get_world_counter()
0x0000000000007221
julia> macro m() 2 end
@m (macro with 1 method)
julia> Base.get_world_counter()
0x0000000000007222
julia> Base.invoke_in_world(wa, Meta.lower, Main, :(@m))
2@c42f Is it possible to specify which worldage macro expansion should use? |
|
It doesn't seem so: https://github.com/JuliaLang/julia/blob/949caad5f13e60c62a2b774b93057754afc3f0b4/src/ast.c#L971 It would probably make sense to have the ability to pass through an additional worldage argument in |
|
So does |
|
Yes, together with JuliaLang/julia#37902, this works: diff --git a/src/legacy_loading.jl b/src/legacy_loading.jl
index e310f22..5a6448f 100644
--- a/src/legacy_loading.jl
+++ b/src/legacy_loading.jl
@@ -156,7 +156,7 @@ function parse_pkg_files(id::PkgId)
# To reduce compiler latency, use runtime dispatch for `queue_includes!`.
# `queue_includes!` requires compilation of the whole parsing/expression-splitting infrastructure,
# and it's better to wait to compile it until we actually need it.
- Base.invokelatest(queue_includes!, pkgdata, id)
+ Base.invoke_in_world(worldage[], queue_includes!, pkgdata, id)
return pkgdata
end
diff --git a/src/loading.jl b/src/loading.jl
index db8d0b2..0764985 100644
--- a/src/loading.jl
+++ b/src/loading.jl
@@ -48,7 +48,7 @@ function parse_pkg_files(id::PkgId)
# To reduce compiler latency, use runtime dispatch for `queue_includes!`.
# `queue_includes!` requires compilation of the whole parsing/expression-splitting infrastructure,
# and it's better to wait to compile it until we actually need it.
- Base.invokelatest(queue_includes!, pkgdata, id)
+ Base.invoke_in_world(worldage[], queue_includes!, pkgdata, id)
return pkgdata
end
diff --git a/src/lowered.jl b/src/lowered.jl
index e754c6b..d3812e6 100644
--- a/src/lowered.jl
+++ b/src/lowered.jl
@@ -128,6 +128,8 @@ function methods_by_execution(mod::Module, ex::Expr; kwargs...)
return methodinfo, docexprs, frame
end
+_lower(m::Module, ex, world::UInt) = ccall(:jl_expand_in_world, Any, (Any, Module, Cstring, Cint, Csize_t), ex, m, "none", 0, world)
+
"""
methods_by_execution!(recurse=JuliaInterpreter.Compiled(), methodinfo, docexprs, mod::Module, ex::Expr;
mode=:eval, disablebp=true, skip_include=mode!==:eval, always_rethrow=false)
@@ -175,7 +177,7 @@ The other keyword arguments are more straightforward:
function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, mod::Module, ex::Expr;
mode::Symbol=:eval, disablebp::Bool=true, always_rethrow::Bool=false, kwargs...)
mode ∈ (:sigs, :eval, :evalmeth, :evalassign) || error("unsupported mode ", mode)
- lwr = Meta.lower(mod, ex)
+ lwr = _lower(mod, ex, worldage[])
isa(lwr, Expr) || return nothing, nothing
if lwr.head === :error || lwr.head === :incomplete
error("lowering returned an error, ", lwr)
diff --git a/src/packagedef.jl b/src/packagedef.jl
index 441c25a..c053b8a 100644
--- a/src/packagedef.jl
+++ b/src/packagedef.jl
@@ -198,6 +198,11 @@ const silence_pkgs = Set{Symbol}()
const depsdir = joinpath(dirname(@__DIR__), "deps")
const silencefile = Ref(joinpath(depsdir, "silence.txt")) # Ref so that tests don't clobber
+"""
+ world age
+"""
+const worldage = Ref{Union{Nothing,UInt}}(nothing)
+
##
## The inputs are sets of expressions found in each file.
## Some of those expressions will generate methods which are identified via their signatures.
@@ -1178,7 +1183,7 @@ end
# This uses invokelatest not for reasons of world age but to ensure that the call is made at runtime.
# This allows `revise_first` to be compiled without compiling `revise` itself, and greatly
# reduces the overhead of using Revise.
-revise_first(ex) = Expr(:toplevel, :(isempty($revision_queue) || Base.invokelatest($revise)), ex)
+revise_first(ex) = Expr(:toplevel, :(isempty($revision_queue) || Base.invoke_in_world($(worldage[]), $revise)), ex)
@noinline function run_backend(backend)
while true
@@ -1277,6 +1282,7 @@ function init_worker(p)
end
function __init__()
+ worldage[] = Base.get_world_counter()
run_on_worker = get(ENV, "JULIA_REVISE_WORKER_ONLY", "0")
if !(myid() == 1 || run_on_worker == "1")
return nothing
diff --git a/src/pkgs.jl b/src/pkgs.jl
index 95fbaa6..fb515f1 100644
--- a/src/pkgs.jl
+++ b/src/pkgs.jl
@@ -171,7 +171,7 @@ function maybe_add_includes_to_pkgdata!(pkgdata::PkgData, file::AbstractString,
parse_source!(fi.modexsigs, fullfile, mod)
if eval_now
# Use runtime dispatch to reduce latency
- Base.invokelatest(instantiate_sigs!, fi.modexsigs; mode=:eval)
+ Base.invoke_in_world(worldage[], instantiate_sigs!, fi.modexsigs; mode=:eval)
end
end
# Add to watchlist
@@ -237,7 +237,7 @@ function _add_require(sourcefile, modcaller, idmod, modname, expr)
end
end
if complex
- Base.invokelatest(eval_require_now, pkgdata, fileidx, filekey, sourcefile, modcaller, expr)
+ Base.invoke_in_world(worldage[], eval_require_now, pkgdata, fileidx, filekey, sourcefile, modcaller, expr)
end
finally
unlock(requires_lock)
diff --git a/src/utils.jl b/src/utils.jl
index 785c4ac..7b97483 100644
--- a/src/utils.jl
+++ b/src/utils.jl
@@ -184,6 +184,7 @@ println_maxsize(io::IO, args...; kwargs...) = printf_maxsize(println, io, args..
# Trimming backtraces
function trim_toplevel!(bt)
+ return bt
n = itoplevel = length(bt)
for (i, t) in enumerate(bt)
sfs = StackTraces.lookup(t) |
|
Wait, I spoke too soon! After trying this again, I get the following output: It doesn't seem to be fatal, and I can use the REPL just fine without any error messages afterwards. So this is definitely an improvement, but it looks like some finalizers are still run in the wrong worldage. |
That's what happens to me in the original...but I get a crash when I exit Julia. But that's without |
|
With |
|
I wonder if we can find out whether these are Revise-dependent. It's too bad this involves an inner constructor, otherwise we could check whether just |
|
I applied this patch to my Julia build, in hope to get more info about these errors in the finalizers: diff --git a/base/gcutils.jl b/base/gcutils.jl
index 51e3943877..93e8623696 100644
--- a/base/gcutils.jl
+++ b/base/gcutils.jl
@@ -45,8 +45,19 @@ function finalizer(@nospecialize(f), @nospecialize(o))
if !ismutable(o)
error("objects of type ", typeof(o), " cannot be finalized")
end
+ function _f(o)
+ try
+ f(o)
+ catch e
+ print("o = ")
+ show(stderr, o)
+ println()
+ Base.showerror(stderr, e)
+ Base.show_backtrace(stderr, backtrace())
+ end
+ end
ccall(:jl_gc_add_finalizer_th, Cvoid, (Ptr{Cvoid}, Any, Any),
- Core.getptls(), o, f)
+ Core.getptls(), o, _f)
return o
endIt didn't work, but I now got this error: julia> Revise.track(Base)
julia> 1+1
o = error in running finalizer: ErrorException("task switch not allowed from inside gc finalizer")
o = error in running finalizer: ErrorException("task switch not allowed from inside gc finalizer")
o = error in running finalizer: ErrorException("task switch not allowed from inside gc finalizer")
o = error in running finalizer: ErrorException("task switch not allowed from inside gc finalizer")
o = error in running finalizer: ErrorException("task switch not allowed from inside gc finalizer")
2I might just be reading this completely wrong, but it leads me to suspect that this may have something to do with tasks Revise spawns? Do you have any ideas how to further debug this? |
const finalizer_backtraces = []
function finalizer(...
try
f(o)
catch e
push!(finalizer_backtraces, backtrace())
...
end
...and then look at them later. (EDIT: the task switch comes from the fact that you're trying to print, so the remedy is to avoid printing. Storing the data somewhere lets you look at it later.) |
|
Ok, I think I'm a bit out of ideas: diff --git a/base/gcutils.jl b/base/gcutils.jl
index 51e3943877..fc674766f6 100644
--- a/base/gcutils.jl
+++ b/base/gcutils.jl
@@ -4,6 +4,8 @@
==(w::WeakRef, v) = isequal(w.value, v)
==(w, v::WeakRef) = isequal(w, v.value)
+const finalizer_bts = Array{Any,1}()
+
"""
finalizer(f, x)
@@ -45,8 +47,16 @@ function finalizer(@nospecialize(f), @nospecialize(o))
if !ismutable(o)
error("objects of type ", typeof(o), " cannot be finalized")
end
+ function _f(o)
+ try
+ f(o)
+ catch e
+ push!(Base.finalizer_bts, backtrace())
+ rethrow()
+ end
+ end
ccall(:jl_gc_add_finalizer_th, Cvoid, (Ptr{Cvoid}, Any, Any),
- Core.getptls(), o, f)
+ Core.getptls(), o, _f)
return o
endMy best guess is that it's somehow due to bootstrapping, so we actually have two separate |
|
Nice progress! Could the finalization problem be due to the GC always running finalizers in the latest world? See here: https://github.com/JuliaLang/julia/blob/7d3dac44dc917a215607bfa1a6054a21846f02a7/src/gc.c#L275 Combine this with
You can probably work around this by hackily calling blocking IO from the C library rather than using Julia's non-blocking IO. For example, julia> kill(a) = @ccall jl_safe_printf("finalizing a %s\n %s"::Cstring;
repr(typeof(a))::Cstring,
repr(a)::Cstring)::Cvoid
kill (generic function with 1 method)
julia> let
finalizer(kill, [1,2])
nothing
end
GC.gc()
finalizing a Vector{Int64}
[1, 2] |
|
Ok, we've got an object and something resembling a stacktrace (although I don't think this stacktrace is particularly helpful, because it doesn't tell us anything about where the finalizer was initially created, and for some reason, it's also cut off): I suspect the finalizer comes from I obtained this using the following patch on top of my `jl_expand_in_world` branch:
diff --git a/base/gcutils.jl b/base/gcutils.jl
index 51e3943877..5da0fef06a 100644
--- a/base/gcutils.jl
+++ b/base/gcutils.jl
@@ -45,8 +45,24 @@ function finalizer(@nospecialize(f), @nospecialize(o))
if !ismutable(o)
error("objects of type ", typeof(o), " cannot be finalized")
end
+
+ wa = get_world_counter()
+
+ function _f(o)
+ try
+ f(o)
+ catch e
+ invoke_in_world(wa, function()
+ safe_print("$(repr(o)) :: $(typeof(o))\n")
+ safe_print("ERROR: " * sprint(showerror, e) * "\n")
+ safe_print(sprint(show_backtrace, backtrace()) * "\n")
+ end)
+ nothing
+ end
+ end
+
ccall(:jl_gc_add_finalizer_th, Cvoid, (Ptr{Cvoid}, Any, Any),
- Core.getptls(), o, f)
+ Core.getptls(), o, _f)
return o
end
diff --git a/base/refpointer.jl b/base/refpointer.jl
index 67cec0925f..ae994517d8 100644
--- a/base/refpointer.jl
+++ b/base/refpointer.jl
@@ -194,3 +194,5 @@ e.g., intrinsics that are selected based on the address space, or back-ends that
pointers to be identifiable by their types.
"""
Core.LLVMPtr
+
+safe_print(s) = ccall(:jl_safe_printf, Cvoid, (Cstring,), s) |
|
Nice progress! I hate to keep asking, but what happens if you put |
It supports revising workers, e.g., Lines 264 to 269 in ed86caf |
TIL that you can use
Funny you should point me to those exact lines, I think they are the problem: We may need to provide a different finalizer for |
|
This is awesome progress. What a team! |
|
🎉: Look ma, no MethodErrors! 😆 |
|
And thanks for both of your help! I really learned a lot about Julia and Revise internals I didn't know before. |
Steps to reproduce:
Apply this patch to
base/range.jlon the latest Julia master:Run Julia and do:
That should work, right? My best guess is that the compiler is trying to inline a method that Revise just deleted.
The text was updated successfully, but these errors were encountered: