does closure_env_ptr.read() imply we need a forget elsewhere?
Closed this issue · 3 comments
The code currently does closure_env_ptr.read() to extract the closure's environment before doing the call into the callback.
But the read() method is unsafe for a reason; see https://doc.rust-lang.org/std/ptr/fn.read.html#ownership-of-the-returned-value
Namely, you can end up with multiple ownership paths to the same owned data, which can then lead to badness/UB from double-drops etc.
I need to double-check the behavior here, and fix things if necessary.
Okay, confirmed its double-dropping. Lets fix that.
of course, its a total anti-pattern to be putting stuff that expects to be dropping into the closure's environment here, since longjmp'ing out of the closure won't respect those. But longjmp itself is unsafe, and the current code is doing the wrong thing in the absence of any longjmp at all, which is bad (since e.g. call_with_setjmp itself is a a safe function).