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

tests: keep some string vectors alive for references #461

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

benlorenz
Copy link
Contributor

This should avoid intermittent errors like this https://github.com/JuliaInterop/libcxxwrap-julia/actions/runs/12225741316/job/34100102886?pr=177#step:4:1251 in JuliaInterop/libcxxwrap-julia#177.

3: parameterized constructors: Test Failed at /Users/runner/.julia/packages/CxxWrap/VaRGA/test/stdlib.jl:191
3:   Expression: vec == ["a", "b", "c"]
3:    Evaluated: StdString[":5\xe7}\"\0\0\xc8\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\xe015\xe7}\"\0\0\x99", "15\xe7}\"\0\0\x99\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\xa0=5\xe7}\"\0\0\xde\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x10\x04\f\xe5}\"\0\0\b\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0", "=5\xe7}\"\0\0\xde\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x10\x04\f\xe5}\"\0\0\b\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"] == ["a", "b", "c"]

Unless I understand this wrong, a CxxRef only stores a pointer and does not protect anything from being garbage collected, e.g.:

julia> str = StdString("a");

julia> finalizer(str) do x
       ccall(:jl_safe_printf, Cvoid, (Cstring, Cstring), "Finalizing %s.", repr(x))
       end;

julia> strref = CxxRef(str)
CxxRef{StdString}(Ptr{StdString} @0x000000001df15250)

julia> GC.gc()

julia> str = "reassigning string variable";

julia> GC.gc()
Finalizing "a".

julia> strref
CxxRef{StdString}(Ptr{StdString} @0x000000001df15250)

julia> strref[]

[18274] signal 11 (1): Segmentation fault
in expression starting at none:0
_ZNSt17_Function_handlerIFcRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEElEZN5jlcxx3stl11wrap_stringIS5_EEvONS9_11TypeWrapperIT_EEEUlS7_lE1_E9_M_invokeERKSt9_Any_dataS7_Ol at /home/lorenz/.julia/artifacts/dee31c8c121f2be2dd7c7b2ec3d41b2be90cc8c7/lib/libcxxwrap_julia_stl.so (unknown line)
_ZN5jlcxx6detail11CallFunctorIcJRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEElEE5applyEPKvNS_13WrappedCppPtrEl at /home/lorenz/.julia/artifacts/dee31c8c121f2be2dd7c7b2ec3d41b2be90cc8c7/lib/libcxxwrap_julia_stl.so (unknown line)
cxxgetindex at /home/lorenz/.julia/packages/CxxWrap/eWADG/src/CxxWrap.jl:668 [inlined]
codeunit at /home/lorenz/.julia/packages/CxxWrap/eWADG/src/StdLib.jl:38 [inlined]
_thisind_str at ./strings/string.jl:177 [inlined]
thisind at /home/lorenz/.julia/packages/CxxWrap/eWADG/src/StdLib.jl:41 [inlined]
isvalid at /home/lorenz/.julia/packages/CxxWrap/eWADG/src/StdLib.jl:39
prevind at ./strings/basic.jl:519
#show#593 at ./strings/io.jl:232
show at ./strings/io.jl:200
unknown function (ip: 0x7f90b244cb56)
#68 at /cache/build/builder-demeter6-6/julialang/julia-master/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:367
jfptr_YY.68_10032 at /home/lorenz/software/polymake/julia/julia/julia-1.11.1/share/julia/compiled/v1.11/REPL/u0gqU_GYsA8.so (unknown line)
with_repl_linfo at /cache/build/builder-demeter6-6/julialang/julia-master/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:661
jfptr_with_repl_linfo_10162 at /home/lorenz/software/polymake/julia/julia/julia-1.11.1/share/julia/compiled/v1.11/REPL/u0gqU_GYsA8.so (unknown line)
display at /cache/build/builder-demeter6-6/julialang/julia-master/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:353
display at /cache/build/builder-demeter6-6/julialang/julia-master/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:372 [inlined]
display at ./multimedia.jl:340
jfptr_display_13539 at /home/lorenz/software/polymake/julia/julia/julia-1.11.1/share/julia/compiled/v1.11/REPL/u0gqU_GYsA8.so (unknown line)
jl_apply at /cache/build/builder-demeter6-6/julialang/julia-master/src/julia.h:2157 [inlined]
jl_f__call_latest at /cache/build/builder-demeter6-6/julialang/julia-master/src/builtins.c:875
#invokelatest#2 at ./essentials.jl:1055 [inlined]
invokelatest at ./essentials.jl:1052 [inlined]
print_response at /cache/build/builder-demeter6-6/julialang/julia-master/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:409
#70 at /cache/build/builder-demeter6-6/julialang/julia-master/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:378
jfptr_YY.70_10070 at /home/lorenz/software/polymake/julia/julia/julia-1.11.1/share/julia/compiled/v1.11/REPL/u0gqU_GYsA8.so (unknown line)
with_repl_linfo at /cache/build/builder-demeter6-6/julialang/julia-master/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:661
jfptr_with_repl_linfo_10162 at /home/lorenz/software/polymake/julia/julia/julia-1.11.1/share/julia/compiled/v1.11/REPL/u0gqU_GYsA8.so (unknown line)
print_response at /cache/build/builder-demeter6-6/julialang/julia-master/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:376
do_respond at /cache/build/builder-demeter6-6/julialang/julia-master/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:1003
jfptr_do_respond_10225 at /home/lorenz/software/polymake/julia/julia/julia-1.11.1/share/julia/compiled/v1.11/REPL/u0gqU_GYsA8.so (unknown line)
jl_apply at /cache/build/builder-demeter6-6/julialang/julia-master/src/julia.h:2157 [inlined]
jl_f__call_latest at /cache/build/builder-demeter6-6/julialang/julia-master/src/builtins.c:875
#invokelatest#2 at ./essentials.jl:1055 [inlined]
invokelatest at ./essentials.jl:1052 [inlined]
run_interface at /cache/build/builder-demeter6-6/julialang/julia-master/usr/share/julia/stdlib/v1.11/REPL/src/LineEdit.jl:2755
jfptr_run_interface_8706 at /home/lorenz/software/polymake/julia/julia/julia-1.11.1/share/julia/compiled/v1.11/REPL/u0gqU_GYsA8.so (unknown line)
run_frontend at /cache/build/builder-demeter6-6/julialang/julia-master/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:1471
#75 at /cache/build/builder-demeter6-6/julialang/julia-master/usr/share/julia/stdlib/v1.11/REPL/src/REPL.jl:480
jfptr_YY.75_10127 at /home/lorenz/software/polymake/julia/julia/julia-1.11.1/share/julia/compiled/v1.11/REPL/u0gqU_GYsA8.so (unknown line)
jl_apply at /cache/build/builder-demeter6-6/julialang/julia-master/src/julia.h:2157 [inlined]
start_task at /cache/build/builder-demeter6-6/julialang/julia-master/src/task.c:1202
Allocations: 6921842 (Pool: 6921467; Big: 375); GC: 9
Segmentation fault

@barche barche merged commit a4eec02 into JuliaInterop:main Dec 9, 2024
0 of 11 checks passed
@barche
Copy link
Collaborator

barche commented Dec 9, 2024

Thanks, good catch, again :)

@fingolfin
Copy link
Contributor

This keeps causing crashes in our CI. It would be great to have a release with this fix in.

On that front it would be great if there was a 0.16 bugfix release with this patch -- master is already at 0.17 it seems, but to switch to that we need to upgrade multiple packages in coordination which takes time and testing...

@benlorenz
Copy link
Contributor Author

This keeps causing crashes in our CI. It would be great to have a release with this fix in.

On that front it would be great if there was a 0.16 bugfix release with this patch -- master is already at 0.17 it seems, but to switch to that we need to upgrade multiple packages in coordination which takes time and testing...

This change here is just for the CxxWrap tests.

The crashes should be fixed by JuliaInterop/libcxxwrap-julia#177, we could probably create a libcxxwrap-julia 0.13.3 on Yggdrasil adding these two commits as patches, they should apply cleanly and I did most of my testing with that version anyway. (This won't require a new CxxWrap release)

@barche
Copy link
Collaborator

barche commented Dec 12, 2024

OK, I'll do a backport for libcxxwrap-julia, since that is the most important fix. I'm postponing the CxxWrap 0.17 release still a bit since this string test failure on macOS nightly is not solved by this PR. I haven't figured it out completely yet, but I think the array of string references actually contains references to a temporary copy of the strings.

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.

3 participants