search

stev47 / StaticKernels.jl

Julia-native non-allocating kernel operations on arrays
Other
25 stars 0 forks source link

`Window` type is not memory safe #2

Closed JeffBezanson closed 4 years ago

JeffBezanson commented 4 years ago

https://github.com/stev47/StaticKernels.jl/blob/d08b6025425973718e8c9d3fdf96394c07415580/src/types.jl#L42

This is not a valid use of Ptr --- it will drop references to a and cause crashes. Please do not do this. I had to lie down for five minutes after seeing this code.

stev47 commented 4 years ago

Yes, I am aware of this and taking the risk in the same way UnsafeArrays does (don't look at it if you do not want to lie down for another five minutes). It is another instance of JuliaLang/julia#14955 User-facing code is safe as long as one does not store the window references. All looping operations use GC.@preserve.

But if you have a better suggestion to achieve the same behaviour while staying allocation-free, I'm open to suggestions.

JeffBezanson commented 4 years ago

Fortunately this should be fixed in julia 1.5 (https://github.com/JuliaLang/julia/pull/34126). At least the name of UnsafeArrays is pretty clear :)

stev47 commented 4 years ago

Thank you, that looks promising indeed! You are right, I'll add an appropriate warning for now.

RoyiAvital commented 3 years ago

Is it closed because a warning was added or because it was reimplemented safely?

stev47 commented 3 years ago

Yes it was implemented safely as soon as I realized the workaround wasn't even necessary in julia 1.4. Since julia 1.5 the situation only got better. Also the referenced commit b884686e27a0b5ca20a5b47b727e34faf9d8e3a8 above is titled: "make Window memory safe"