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

sleftv conversion: protect some more julia variables from intermediate garbage collections #749

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

benlorenz
Copy link
Member

This should fix one of the crashes observed in oscar-system/Oscar.jl#3184. I did some rr tracing and got a run where garbage collection was triggered by the jl_box_voidpointer call which then killed the array where the pointer was supposed to be stored.
The crash then happens when julia tries to compile a method instance for a type signature containing an invalid datatype pointer.

For testing it crashes pretty reliably with an explicit garbage collection in the middle:

jl_value_t * get_julia_type_from_sleftv(leftv ret)
{
  jl_array_t * result = jl_alloc_array_1d(jl_array_any_type, 3); 
  jl_array_ptr_set(result, 0, jl_false);
  jl_array_ptr_set(result, 1, jl_box_voidpointer(ret->data));
  jl_gc_collect(JL_GC_FULL);
  ret->data = 0;
  jl_array_ptr_set(result, 2, jl_box_int64(ret->Typ()));
  ret->rtyp = 0;
  return reinterpret_cast<jl_value_t *>(result);
}

Once the push is added this crash disappears.

I want to let it run for a bit longer without the explicit collect to make sure the original crash doesn't reappear.

The second JL_GC_PUSH2 change in lookup_singular_library_symbol_wo_rng is unrelated to the crashes but I think this is necessary as well.

Example backtrace with the explicit collect call:
[9856] signal (11.1): Segmentation fault
in expression starting at /home/datastore/lorenz/software/julia/Oscar.jl/test/AlgebraicGeometry/ToricVarieties/toric_schemes.jl:1
jl_object_id__cold at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/builtins.c:455
type_hash at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/jltypes.c:1575
typekey_hash at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/jltypes.c:1605
jl_precompute_memoized_dt at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/jltypes.c:1685
inst_datatype_inner at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/jltypes.c:2081
jl_inst_arg_tuple_type at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/jltypes.c:2176
arg_type_tuple at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/gf.c:2232 [inlined]
jl_lookup_generic_ at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/gf.c:3020 [inlined]
ijl_apply_generic at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/gf.c:3072
low_level_caller_rng at /home/datastore/lorenz/software/julia/Singular.jl/src/caller.jl:378
primdecGTZ at /home/datastore/lorenz/software/julia/Singular.jl/src/Meta.jl:45
unknown function (ip: 0x14dcb5dadb69)
_jl_invoke at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/gf.c:2875 [inlined]
ijl_apply_generic at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/gf.c:3076
#_compute_primary_decomposition#320 at /home/datastore/lorenz/software/julia/Oscar.jl/src/Rings/mpoly-ideals.jl:578
_compute_primary_decomposition at /home/datastore/lorenz/software/julia/Oscar.jl/src/Rings/mpoly-ideals.jl:566 [inlined]
#313 at /home/datastore/lorenz/software/julia/Oscar.jl/src/Rings/mpoly-ideals.jl:541 [inlined]
get! at ./dict.jl:479
get_attribute! at /home/datastore/lorenz/software/julia/depot/packages/AbstractAlgebra/R29qD/src/Attributes.jl:230 [inlined]
#primary_decomposition#312 at /home/datastore/lorenz/software/julia/Oscar.jl/src/Rings/mpoly-ideals.jl:540 [inlined]
primary_decomposition at /home/datastore/lorenz/software/julia/Oscar.jl/src/Rings/mpoly-ideals.jl:538 [inlined]
connected_components at /home/datastore/lorenz/software/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/AffineSchemes/Objects/Methods.jl:183
#_is_projective_without_denominators#7140 at /home/datastore/lorenz/software/julia/Oscar.jl/experimental/Schemes/ProjectiveModules.jl:95
_is_projective_without_denominators at /home/datastore/lorenz/software/julia/Oscar.jl/experimental/Schemes/ProjectiveModules.jl:71 [inlined]
__compute_is_smooth__ at /home/datastore/lorenz/software/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/AffineSchemes/Objects/Properties.jl:705
#3793 at /home/datastore/lorenz/software/julia/depot/packages/AbstractAlgebra/R29qD/src/Attributes.jl:357
get! at ./dict.jl:479
unknown function (ip: 0x14dcb5d2b200)
_jl_invoke at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/gf.c:2875 [inlined]
ijl_invoke at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/gf.c:2901
unknown function (ip: 0x14dcd8ffe839)
unknown function (ip: 0x14dcd8ffe80b)
get_attribute! at /home/datastore/lorenz/software/julia/depot/packages/AbstractAlgebra/R29qD/src/Attributes.jl:230 [inlined]
is_smooth at /home/datastore/lorenz/software/julia/Oscar.jl/src/AlgebraicGeometry/Schemes/AffineSchemes/Objects/Properties.jl:699
unknown function (ip: 0x14dcd8ffe745)
_jl_invoke at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/gf.c:2875 [inlined]
ijl_apply_generic at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/gf.c:3076
jl_apply at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/julia.h:1982 [inlined]
do_call at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/interpreter.c:126
eval_value at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/interpreter.c:223
eval_stmt_value at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/interpreter.c:174 [inlined]
eval_body at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/interpreter.c:621
eval_body at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/interpreter.c:544
eval_body at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/interpreter.c:544
eval_body at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/interpreter.c:544
eval_body at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/interpreter.c:544
eval_body at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/interpreter.c:544
jl_interpret_toplevel_thunk at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/interpreter.c:775
jl_toplevel_eval_flex at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/toplevel.c:934
jl_toplevel_eval_flex at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/toplevel.c:877
ijl_toplevel_eval_in at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/toplevel.c:985
eval at ./boot.jl:385 [inlined]
include_string at ./loading.jl:2070
_jl_invoke at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/gf.c:2875 [inlined]
ijl_apply_generic at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/gf.c:3076
_include at ./loading.jl:2130
include at ./Base.jl:496 [inlined]
macro expansion at ./timing.jl:503 [inlined]
#_timed_include#21 at /home/datastore/lorenz/software/julia/Oscar.jl/src/utils/tests.jl:5
_timed_include at /home/datastore/lorenz/software/julia/Oscar.jl/src/utils/tests.jl:1 [inlined]
_timed_include at /home/datastore/lorenz/software/julia/Oscar.jl/src/utils/tests.jl:1 [inlined]
#test_module#27 at /home/datastore/lorenz/software/julia/Oscar.jl/src/utils/tests.jl:151
test_module at /home/datastore/lorenz/software/julia/Oscar.jl/src/utils/tests.jl:119 [inlined]
#29 at /home/datastore/lorenz/software/julia/Oscar.jl/test/runtests.jl:134 [inlined]
#1023 at ./asyncmap.jl:94
unknown function (ip: 0x14dcd8f50979)
_jl_invoke at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/gf.c:2875 [inlined]
ijl_apply_generic at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/gf.c:3076
jl_apply at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/julia.h:1982 [inlined]
do_apply at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/builtins.c:768
#1039 at ./asyncmap.jl:228
unknown function (ip: 0x14dcd8f4cd62)
_jl_invoke at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/gf.c:2875 [inlined]
ijl_apply_generic at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/gf.c:3076
jl_apply at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/julia.h:1982 [inlined]
start_task at /home/datastore/lorenz/software/julia/jv/julia-110-assert/src/task.c:1238
Allocations: 134206885 (Pool: 134090342; Big: 116543); GC: 73

@benlorenz benlorenz changed the title type conversion: protect some more julia variables from intermediate garbage collections sleftv conversion: protect some more julia variables from intermediate garbage collections Jan 18, 2024
Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch

@benlorenz benlorenz marked this pull request as ready for review January 19, 2024 12:57
@benlorenz
Copy link
Member Author

So far I haven't seen any further crashes in about 100 iterations. I will let it continue for a while but from my point of view we can merge (+build) this now. Either with a new libsingular_julia version, or just as a rebuild (+1) of the previous build.

@benlorenz benlorenz merged commit 5919a47 into master Jan 19, 2024
13 of 14 checks passed
@benlorenz benlorenz deleted the bl/gcpush branch January 19, 2024 13:45
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.

2 participants