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

Window type is not memory safe #2

Closed
JeffBezanson opened this issue Apr 21, 2020 · 5 comments
Closed

Window type is not memory safe #2

JeffBezanson opened this issue Apr 21, 2020 · 5 comments

Comments

@JeffBezanson
Copy link

parent_ptr::Ptr{T}

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
Copy link
Owner

stev47 commented Apr 21, 2020

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
Copy link
Author

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

@stev47
Copy link
Owner

stev47 commented Apr 21, 2020

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

@stev47 stev47 closed this as completed in b884686 Apr 21, 2020
@RoyiAvital
Copy link

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

@stev47
Copy link
Owner

stev47 commented May 4, 2021

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 b884686 above is titled: "make Window memory safe"

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

No branches or pull requests

3 participants