Skip to content

Commit

Permalink
Don't try to allocate new binding partitions from static show (#56298)
Browse files Browse the repository at this point in the history
In particular static show is used inside the GC for profiling, which
showed up as a segfault on CI, e.g. in

https://buildkite.com/julialang/julia-master/builds/41407#0192b628-47f3-49f9-a081-cd2708eb6121.

GC check didn't catch it because that file is explicitly exempt:
https://github.com/JuliaLang/julia/blob/master/src/Makefile#L504
  • Loading branch information
Keno authored Oct 23, 2024
1 parent 73b85cf commit be0ce9d
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -1848,6 +1848,7 @@ JL_DLLEXPORT jl_sym_t *jl_tagged_gensym(const char *str, size_t len);
JL_DLLEXPORT jl_sym_t *jl_get_root_symbol(void);
JL_DLLEXPORT jl_value_t *jl_get_binding_value(jl_binding_t *b JL_PROPAGATES_ROOT);
JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_const(jl_binding_t *b JL_PROPAGATES_ROOT);
JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_resolved_and_const(jl_binding_t *b JL_PROPAGATES_ROOT) JL_NOTSAFEPOINT;
JL_DLLEXPORT jl_value_t *jl_declare_const_gf(jl_binding_t *b, jl_module_t *mod, jl_sym_t *name);
JL_DLLEXPORT jl_method_t *jl_method_def(jl_svec_t *argdata, jl_methtable_t *mt, jl_code_info_t *f, jl_module_t *module);
JL_DLLEXPORT jl_code_info_t *jl_code_for_staged(jl_method_instance_t *linfo, size_t world, jl_code_instance_t **cache);
Expand Down
21 changes: 21 additions & 0 deletions src/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ jl_binding_partition_t *jl_get_binding_partition(jl_binding_t *b, size_t world)
return bpart;
jl_binding_partition_t *new_bpart = new_binding_partition();
jl_atomic_store_relaxed(&new_bpart->next, bpart);
jl_gc_wb_fresh(new_bpart, bpart);
if (bpart)
new_bpart->min_world = jl_atomic_load_relaxed(&bpart->max_world) + 1;
jl_atomic_store_relaxed(&new_bpart->max_world, max_world);
Expand Down Expand Up @@ -350,6 +351,26 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_const(jl_binding_t *b)
return decode_restriction_value(pku);
}

JL_DLLEXPORT jl_value_t *jl_get_binding_value_if_resolved_and_const(jl_binding_t *b)
{
// Unlike jl_get_binding_value_if_const this doesn't try to allocate new binding partitions if they
// don't already exist, making this JL_NOTSAFEPOINT.
if (!b)
return NULL;
jl_binding_partition_t *bpart = jl_atomic_load_relaxed(&b->partitions);
if (!bpart)
return NULL;
size_t max_world = jl_atomic_load_relaxed(&bpart->max_world);
if (bpart->min_world > jl_current_task->world_age || jl_current_task->world_age > max_world)
return NULL;
jl_ptr_kind_union_t pku = jl_atomic_load_relaxed(&bpart->restriction);
if (jl_bkind_is_some_guard(decode_restriction_kind(pku)))
return NULL;
if (!jl_bkind_is_some_constant(decode_restriction_kind(pku)))
return NULL;
return decode_restriction_value(pku);
}

JL_DLLEXPORT jl_value_t *jl_bpart_get_restriction_value(jl_binding_partition_t *bpart)
{
jl_ptr_kind_union_t pku = jl_atomic_load_relaxed(&bpart->restriction);
Expand Down
2 changes: 1 addition & 1 deletion src/rtutils.c
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,7 @@ static int is_globname_binding(jl_value_t *v, jl_datatype_t *dv) JL_NOTSAFEPOINT
jl_sym_t *globname = dv->name->mt != NULL ? dv->name->mt->name : NULL;
if (globname && dv->name->module) {
jl_binding_t *b = jl_get_module_binding(dv->name->module, globname, 0);
jl_value_t *bv = jl_get_binding_value_if_const(b);
jl_value_t *bv = jl_get_binding_value_if_resolved_and_const(b);
// The `||` makes this function work for both function instances and function types.
if (bv && (bv == v || jl_typeof(bv) == v))
return 1;
Expand Down

0 comments on commit be0ce9d

Please sign in to comment.