JuliaGraphics / Luxor.jl

Simple drawings using vector graphics; Cairo "for tourists!"
http://juliagraphics.github.io/Luxor.jl/
Other
588 stars 71 forks source link

Exception: EXCEPTION_ACCESS_VIOLATION #250

Closed hustf closed 1 year ago

hustf commented 1 year ago

This is a temporary issue / note. If crashes keep occuring, I'll post proper, reproducible code here or in Cairo.jl. The issue probably belongs elsewhere in the end. I do not believe this is the fault of the latest changes in master, though my function encompass is related to testing it.

Crashes have occured three or five times in different circumstances while giving the latest master a whirl. As I was working in VSCode, the detailed errors were lost as Julia crashed. This time, I opened a REPL terminal in order to take notes and dump the output here.

I suspect the crashes are related to calling getmatrix() in very rapid succession. I'm not actually changing the matrix. I call getmatrix() in order to convert points to 'world coordinates'. It may be advisable to store the transformation matrix while mapping multiple points.

Please submit a bug report with steps to reproduce this fault, and any error messages that follow (in their entirety). Thanks.
Exception: EXCEPTION_ACCESS_VIOLATION at 0x4042010 -- cairo_get_matrix at /workspace/srcdir/cairo-1.16.0/src\cairo.c:4130
in expression starting at REPL[5]:1
cairo_get_matrix at /workspace/srcdir/cairo-1.16.0/src\cairo.c:4130
get_matrix at C:\Users\f\.julia\packages\Cairo\smWIA\src\Cairo.jl:1110 [inlined]
getmatrix at C:\Users\f\.julia\dev\Luxor\src\matrix.jl:74
update_INK_EXTENT at C:\Users\f\.julia\environments\infinite_source\issue150_3.jl:72
encompass at C:\Users\f\.julia\environments\infinite_source\issue150_3.jl:124
_broadcast_getindex_evalf at .\broadcast.jl:670 [inlined]
_broadcast_getindex at .\broadcast.jl:643 [inlined]
#29 at .\broadcast.jl:1075
ntuple at .\ntuple.jl:49 [inlined]
copy at .\broadcast.jl:1075 [inlined]
materialize at .\broadcast.jl:860 [inlined]
encompass at C:\Users\f\.julia\environments\infinite_source\issue150_3.jl:125

Version info:

PS C:\Users\f\.julia\environments\infinite_source> julia --project=.

(@infinite_source) pkg> status
Status `C:\Users\f\.julia\environments\infinite_source\Project.toml`
  [159f3aea] Cairo v1.0.5
⌃ [cd3eb016] HTTP v1.6.0
  [ae8d54c2] Luxor v3.6.0 `C:\Users\f\.julia\dev\Luxor`
  [1fd47b50] QuadGK v2.6.0
  [295af30f] Revise v3.4.0
⌃ [36c8627f] Pango_jll v1.42.4+10 ⚲
Info Packages marked with ⌃ have new versions available and may be upgradable.

julia> VERSION
v"1.8.3"
hustf commented 1 year ago

Minimal example with a clear cause - trying to access a non-existing drawing surface:

Works

julia> begin
       println(Threads.nthreads())
       using Luxor
       Drawing()
       rotate(10*π/180)
       end
12

Crashes every time

PS C:\Users\f\.julia\environments\infinite_source> julia --project=.
               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.8.3 (2022-11-14)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> begin
       println(Threads.nthreads())
       using Luxor
       rotate(10*π/180)
       end
12

Please submit a bug report with steps to reproduce this fault, and any error messages that follow (in their entirety). Thanks.
Exception: EXCEPTION_ACCESS_VIOLATION at 0x3650875 -- cairo_rotate at /workspace/srcdir/cairo-1.16.0/src\cairo.c:1468
in expression starting at REPL[1]:1
cairo_rotate at /workspace/srcdir/cairo-1.16.0/src\cairo.c:1468
rotate at C:\Users\f\.julia\packages\Cairo\smWIA\src\Cairo.jl:680 [inlined]
rotate at C:\Users\f\.julia\dev\Luxor\src\basics.jl:607
unknown function (ip: 000001eefc735d2a)
jl_apply at C:/workdir/src\julia.h:1839 [inlined]
<....etc...>
hustf commented 1 year ago

Older versions of Luxor would give a nice warning:

(@oldLuxor) pkg> st
Status `C:\Users\f\.julia\environments\oldLuxor\Project.toml`
⌃ [ae8d54c2] Luxor v2.8.0 ⚲
Info Packages marked with ⌃ have new versions available and may be upgradable.

julia> begin
       println(Threads.nthreads())
       using Luxor
       rotate(10*π/180)
       end
12
ERROR: There is no current drawing.
Stacktrace:
 [1] error(s::String)
   @ Base .\error.jl:35
 [2] get_current_cr()
   @ Luxor C:\Users\f\.julia\packages\Luxor\XSNgj\src\drawings.jl:53
 [3] rotate(a::Float64)
   @ Luxor C:\Users\f\.julia\packages\Luxor\XSNgj\src\basics.jl:534
 [4] top-level scope
   @ REPL[2]:4

caused by: BoundsError: attempt to access 0-element Vector{Drawing} at index [1]
Stacktrace:
 [1] getindex
   @ .\array.jl:924 [inlined]
 [2] get_current_cr()
   @ Luxor C:\Users\frohu_h4g8g6y\.julia\packages\Luxor\XSNgj\src\drawings.jl:51
 [3] rotate(a::Float64)
   @ Luxor C:\Users\frohu_h4g8g6y\.julia\packages\Luxor\XSNgj\src\basics.jl:534
 [4] top-level scope
   @ REPL[2]:4
hustf commented 1 year ago

I believe Luxor.get_current_cr() would be the place for an assertion?

cormullion commented 1 year ago

Let's hope @oheil knows the answer! 😂

hustf commented 1 year ago

One proposal is

@assert c.ptr !== Ptr{Nothing}() "There is no current drawing context."

But that would need to be place outside of the current try..catch structure as an extra precation.

cormullion commented 1 year ago

I don't want to use @assert though. Manual says:

An assert might be disabled at various optimization levels. Assert should therefore only be used as a debugging tool

oheil commented 1 year ago

Looking into it.

oheil commented 1 year ago

Yes, a check needs to be there and, according to the MWE, it needs to break to flow:

function get_current_cr()
    try
        d=getfield(_current_drawing()[_current_drawing_index()], :cr)
        if d.ptr == Ptr{Nothing}()
            error("There is no current drawing.")
        end
        return d
    catch
        error("There is no current drawing.")
    end
end

Ok, like that? PR incoming...

oheil commented 1 year ago

Wait, no, above is bad...

oheil commented 1 year ago

Actually this is the result of a remnant of SnoopPrecompile. I can't find out why there exists a Drawing remnant because if I do:

#include("precompile.jl")

in Luxor.jl, and do it manually in the REPL

include("precompile.jl")

there is no remnant and calling:

julia> using Luxor
[ Info: Precompiling Luxor [ae8d54c2-7ccd-5906-9d76-62fc9837b5bc]

julia> include("src\\precompile.jl")
[ Info: SnoopPrecompile is analyzing Luxor.jl code...

julia> cr=Luxor.get_current_cr()
ERROR: There is no current drawing.

is fine.

oheil commented 1 year ago

Perhaps because

Statements that occur inside a @precompile_all_calls block are executed only if the package is being actively precompiled; it does not run when the package is loaded,

Ok, anyways, if the Drawing is incomplete (CairoContext == C_NULL, CairoSurface == C_NULL) it must be initialized by call to Drawing(...).

oheil commented 1 year ago

This shows the remnant, because the buffer is intact:

julia> using Luxor
[ Info: Precompiling Luxor [ae8d54c2-7ccd-5906-9d76-62fc9837b5bc]
[ Info: SnoopPrecompile is analyzing Luxor.jl code...

julia> d=Luxor._current_drawing()[Luxor._current_drawing_index()]

Opens the "snooping away" PNG.

cormullion commented 1 year ago

I don’t understand much of SnoopPrecompile, but I assumed that adding it would help performance. Perhaps it needs some more attention…

oheil commented 1 year ago

Me neither. Anyways it's an example where a corrupted Drawing remains as if it were fine. Luxor should be as defensive as possible to prevent bad pointers in Cairo. I am looking currently for the best place for a check.

hustf commented 1 year ago

The documentation has some advice on references to external libraries, pre_compilation and __init__(). I read it from time to time. I also brush my teeth two times every day.

oheil commented 1 year ago

Some points there seem to fit. I will do this: 1) Making Luxor more robust for the general case 2) Find the specific caveat for SnoopPrecompile (like global IDs and/or Dicts...) 3) Update this thread with my findings 4) PR when ready 5) brushing my teeth

oheil commented 1 year ago

Skipped 3) PR is https://github.com/JuliaGraphics/Luxor.jl/pull/251

I wish you a Merry Christmas and a Happy New Year !!!

hustf commented 1 year ago

You too, great work that you do for the community lately! I'll mention it to the Christmas authorities.

oheil commented 1 year ago

Resolved in master. Issue can be closed after @hustf confirmed.

hustf commented 1 year ago

I'm trying to break this fix as hard as I can! It's not easy, but wait and see!

oheil commented 1 year ago

Perfect! No good software without extreme testing!

cormullion commented 1 year ago

Make sure you only test @oheil's code then 😂😂😂

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

oheil commented 1 year ago

Resolved in master. Issue should be closed. If anything happens it's better to create new issues. The existing issues are getting cluttered a bit.