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

Allow freezing of Core.MethodTables #56143

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions base/runtime_internals.jl
Original file line number Diff line number Diff line change
Expand Up @@ -966,6 +966,19 @@ typeintersect(@nospecialize(a), @nospecialize(b)) = (@_total_meta; ccall(:jl_typ
morespecific(@nospecialize(a), @nospecialize(b)) = (@_total_meta; ccall(:jl_type_morespecific, Cint, (Any, Any), a::Type, b::Type) != 0)
morespecific(a::Method, b::Method) = ccall(:jl_method_morespecific, Cint, (Any, Any), a, b) != 0

"""
morespecific!(m::Method)

Disallow adding methods more specific than `m`.
"""
function morespecific!(m::Method)
mt = get_methodtable(m)
# check that all Methods in this table are morespecific than m
# so we might avoid disabling a table that might get used for more than just subsets of m
all(m2 -> m === m2 || morespecific(m2, m), MethodList(mt)) || error("unsupported Method to disable")
setfield!(mt, :frozen, 0x1, :monotonic)
end

"""
fieldoffset(type, i)

Expand Down
2 changes: 1 addition & 1 deletion src/datatype.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ JL_DLLEXPORT jl_methtable_t *jl_new_method_table(jl_sym_t *name, jl_module_t *mo
mt->backedges = NULL;
JL_MUTEX_INIT(&mt->writelock, "methodtable->writelock");
mt->offs = 0;
mt->frozen = 0;
jl_atomic_store_relaxed(&mt->frozen, 0);
return mt;
}

Expand Down
4 changes: 2 additions & 2 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ jl_datatype_t *jl_mk_builtin_func(jl_datatype_t *dt, const char *name, jl_fptr_a
(jl_value_t*)mi, 1, ~(size_t)0);
jl_typemap_insert(&mt->cache, (jl_value_t*)mt, newentry, 0);

mt->frozen = 1;
jl_atomic_store_relaxed(&mt->frozen, 1);
JL_GC_POP();
return dt;
}
Expand Down Expand Up @@ -772,7 +772,7 @@ static int reset_mt_caches(jl_methtable_t *mt, void *env)
{
// removes all method caches
// this might not be entirely safe (GC or MT), thus we only do it very early in bootstrapping
if (!mt->frozen) { // make sure not to reset builtin functions
if (!jl_atomic_load_relaxed(&mt->frozen)) { // make sure not to reset frozen functions
jl_atomic_store_release(&mt->leafcache, (jl_genericmemory_t*)jl_an_empty_memory_any);
jl_atomic_store_release(&mt->cache, jl_nothing);
}
Expand Down
4 changes: 2 additions & 2 deletions src/jltypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -3038,9 +3038,9 @@ void jl_init_types(void) JL_GC_DISABLED
jl_methtable_type->name->names = jl_perm_symsvec(11, "name", "defs",
"leafcache", "cache", "max_args",
"module", "backedges",
"", "", "offs", "");
"", "", "offs", "frozen");
const static uint32_t methtable_constfields[1] = { 0x00000020 }; // (1<<5);
const static uint32_t methtable_atomicfields[1] = { 0x0000001e }; // (1<<1)|(1<<2)|(1<<3)|(1<<4);
const static uint32_t methtable_atomicfields[1] = { 0x00000041e }; // (1<<1)|(1<<2)|(1<<3)|(1<<4)|(1<<10);
jl_methtable_type->name->constfields = methtable_constfields;
jl_methtable_type->name->atomicfields = methtable_atomicfields;
jl_precompute_memoized_dt(jl_methtable_type, 1);
Expand Down
2 changes: 1 addition & 1 deletion src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,7 @@ typedef struct _jl_methtable_t {
jl_array_t *backedges; // (sig, caller::MethodInstance) pairs
jl_mutex_t writelock;
uint8_t offs; // 0, or 1 to skip splitting typemap on first (function) argument
uint8_t frozen; // whether this accepts adding new methods
_Atomic(uint8_t) frozen; // whether this accepts adding new methods
} jl_methtable_t;

typedef struct {
Expand Down
4 changes: 2 additions & 2 deletions src/method.c
Original file line number Diff line number Diff line change
Expand Up @@ -1226,8 +1226,8 @@ JL_DLLEXPORT jl_method_t* jl_method_def(jl_svec_t *argdata,
mt = jl_method_table_for(argtype);
if ((jl_value_t*)mt == jl_nothing)
jl_error("Method dispatch is unimplemented currently for this method signature");
if (mt->frozen)
jl_error("cannot add methods to a builtin function");
if (jl_atomic_load_acquire(&mt->frozen))
jl_error("cannot add methods to or modify methods of a frozen function");

assert(jl_is_linenode(functionloc));
jl_sym_t *file = (jl_sym_t*)jl_linenode_file(functionloc);
Expand Down
2 changes: 1 addition & 1 deletion src/staticdata.c
Original file line number Diff line number Diff line change
Expand Up @@ -975,7 +975,7 @@ static void jl_insert_into_serialization_queue(jl_serializer_state *s, jl_value_
}
else if (jl_typetagis(v, jl_typename_type)) {
jl_typename_t *tn = (jl_typename_t*)v;
if (tn->mt != NULL && !tn->mt->frozen) {
if (tn->mt != NULL && !jl_atomic_load_relaxed(&tn->mt->frozen)) {
jl_methtable_t * new_methtable = (jl_methtable_t *)ptrhash_get(&new_methtables, tn->mt);
if (new_methtable != HT_NOTFOUND)
record_field_change((jl_value_t **)&tn->mt, (jl_value_t*)new_methtable);
Expand Down
2 changes: 1 addition & 1 deletion test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2650,7 +2650,7 @@ for f in (:Any, :Function, :(Core.Builtin), :(Union{Nothing, Type}), :(Union{typ
@test_throws ErrorException("Method dispatch is unimplemented currently for this method signature") @eval (::$f)() = 1
end
for f in (:(Core.getfield), :((::typeof(Core.getfield))), :((::Core.IntrinsicFunction)))
@test_throws ErrorException("cannot add methods to a builtin function") @eval $f() = 1
@test_throws ErrorException("cannot add methods to or modify methods of a frozen function") @eval $f() = 1
end

# issue #33370
Expand Down
13 changes: 13 additions & 0 deletions test/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1298,3 +1298,16 @@ end
@test Base.infer_return_type(code_lowered, (Any,Any)) == Vector{Core.CodeInfo}

@test methods(Union{}) == Any[m.method for m in Base._methods_by_ftype(Tuple{Core.TypeofBottom, Vararg}, 1, Base.get_world_counter())] # issue #55187

# disallow adding new methods
f_sealed(x::Int) = x+1
f_sealed(x::Integer) = x+2
@test_throws(
ErrorException("unsupported Method to disable"),
Base.morespecific!(which(f_sealed, (Int,)))
)
Base.morespecific!(which(f_sealed, (Integer,)))
@test_throws(
ErrorException("cannot add methods to or modify methods of a frozen function"),
@eval f_sealed(x::Float64) = x+2
)
fatteneder marked this conversation as resolved.
Show resolved Hide resolved