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

Panics due to wndproc reentrancy #76

Closed
raphlinus opened this issue Jul 11, 2019 · 1 comment
Closed

Panics due to wndproc reentrancy #76

raphlinus opened this issue Jul 11, 2019 · 1 comment

Comments

@raphlinus
Copy link
Contributor

You can provoke a panic in the master branch by running the sample example, selecting File Open from the menu, then doing a dynamic monitor change (for example, unplugging an external monitor). This is because Windows requires the wndproc to be reentrant, and our implementation uses a (panicking) RefCell::borrow_mut() for safety.

There's a Reddit thread which goes into some more detail, including pointers to how winit handles this. There's also some discussion on the Zulip.

The upshot is that we want to refactor the way we handle interior mutability. In particular, druid-shell should handle it, so the WinHandler can be called mutably and not need its own interior mutability. There's also followup work to make the file dialog not hold long-lived borrows, but that's significantly lower priority.

This came up because we need a good way to plumb additional resources, particularly a factory for creating text layouts, to user methods.

raphlinus added a commit that referenced this issue Jul 11, 2019
This makes methods on `WinHandler` mostly `&mut self`, and does
`try_borrow_mut` for this state in the wndproc handler.

WIP, this is Windows only.

Other parts of the work are incomplete:

The UiMain struct has interior mutability it no longer needs, as
the WinHandler methods are called mutably now.

Not all `WindowHandle` methods are moved to `WinCtx` yet.

The text factory story is still incomplete.

Fixes #76 (when it's complete)
raphlinus added a commit that referenced this issue Jul 11, 2019
This makes methods on `WinHandler` mostly `&mut self`, and does
`try_borrow_mut` for this state in the wndproc handler.

WIP, this is Windows only.

Other parts of the work are incomplete:

The UiMain struct has interior mutability it no longer needs, as
the WinHandler methods are called mutably now.

Not all `WindowHandle` methods are moved to `WinCtx` yet.

The text factory story is still incomplete.

Fixes #76 (when it's complete)
@raphlinus
Copy link
Contributor Author

I'm closing this, the panics are fully addressed in the muggle branch, and I'm also satisfied about safety. I'm going to open another issue on finishing the migration from WinHandler to WinCtx.

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

1 participant