win-win icon indicating copy to clipboard operation
win-win copied to clipboard

Panic safety

Open streifmi opened this issue 5 years ago • 2 comments

Since it's currently UB to panic across an FFI boundary (rust-lang/rust#52652), code executed in the window procedure either must not panic or needs wrapping with catch_unwind. You can then either abort on panic or resume it outside the callback.

Winit catches the panic here and then calls resume_unwind in the main loop here. AFAICT druid-shell doesn't handle this at all.

streifmi avatar Feb 23 '20 14:02 streifmi

Thanks for pointing this out! More evidence that this stuff is tricky. We'll get this fixed both here and in druid-shell.

raphlinus avatar Feb 23 '20 14:02 raphlinus

I'm looking at this, and am still not quite sure about a couple things. (I'll probably try to commit a fix here, then port to druid-shell).

First, I'm 100% convinced that the result is still safe. I've been reading the docs on unwind safety and basically am still just as confused as when I started. They refer to interior mutability as a potentially unsafe pattern, and this is a concern, because the given window proc will almost always rely heavily on interior mutability. I'm reluctant to make a change if it's "safety theater" with the result still not sound.

Second, looking at the winit code, I see they have a shared "runner," and this is where the error is stashed. I see two choices here: one is to create such a structure, and require window creation to have a (shared) reference to it, and the other is to make it a global, which would keep the method signatures simpler. A related issue, though, is that if we're not in control of our own runloop, then I don't see how to propagate the panic. That happens on live resize, when a modal dialog is active, or if the window is the guest of a runloop owned by the host (as happens for VST plugins). That last scenario may have no safe answer.

So I'll keep thinking on this, but would also appreciate guidance from someone who understands the issue more deeply than I do.

raphlinus avatar Sep 01 '20 21:09 raphlinus