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

[MacOS] Dialogues freeze when used in winit event loop #1779

Closed
PolyMeilex opened this issue Nov 28, 2020 · 17 comments · Fixed by #2027
Closed

[MacOS] Dialogues freeze when used in winit event loop #1779

PolyMeilex opened this issue Nov 28, 2020 · 17 comments · Fixed by #2027

Comments

@PolyMeilex
Copy link
Contributor

PolyMeilex commented Nov 28, 2020

OS: OSX 10.15.6

Hi I'm working on RustyFileDialog crate, and in the process I found out weird issue related to winit event loop.

(To make it easier to reproduce I'll use nfd2 as an example instead of my impl.)
File dialogs, for example from nfd2 freeze when opened from winit event loop.
Example:

fn main() {
    let el = EventLoop::new();
    let win = WindowBuilder::new().build(&el).unwrap();

    // This dialog works as expected
    nfd2::open_file_dialog(None,None).unwrap();

    el.run(move |event, _, control_flow| {
        match event {
            event::Event::WindowEvent {
                event: event::WindowEvent::MouseInput { .. },
                ..
            } => {
                // This dialog freezes
                nfd2::open_file_dialog(None,None).unwrap();
            }
            _ => {}
        }
    })
}

More info

  • For this test NSOpenPanel is used, and it is opened with runModal (blocking)
  • If I use beginWithCompletionHandler (non-blocking) dialog works as expected
  • SDL2-rs does not have this issue
  • winit window stays responsive, only NSOpenPanel is frozen

GIF

@chrisduerr
Copy link
Contributor

This sounds like you're blocking inside of the event loop? Shouldn't the expected result be that the application then freezes, or what do you expect? The handling of events has been stopped.

@PolyMeilex
Copy link
Contributor Author

PolyMeilex commented Nov 28, 2020

The application should freeze (it did) as MacOS is doing its work with dialog. But for some reason system dialog also freezes.
I'm unable to reproduce this in any way outside of winit event loop.

Or maybe it's my lack of Mac knowledge, is winit event loop somehow tied to system dialogs?

@chrisduerr
Copy link
Contributor

Which macOS version?

@PolyMeilex
Copy link
Contributor Author

Mac OS X 10.15.6 (as stated in the issue)

@chrisduerr
Copy link
Contributor

Mac OS X 10.15.6 (as stated in the issue)

Ah, I missed that. Hm, according to macOS docs it says the dialog boxes should be handled by a completely separate process. Though I'm not sure how exactly you hook them up.

@PolyMeilex
Copy link
Contributor Author

I'm quite confused, I never encountered such a behavior in the SDL world

@PolyMeilex
Copy link
Contributor Author

PolyMeilex commented Dec 5, 2020

Small update on my findings:

When testing without any winit window opened:

  • Mouse events from file dialog cause winit handler to be called
  • Lock on handler callback mutex seems to be the cause of the problem (not deeply tested yet, I may be mistaken)

I located a nice example of this behavior in handle_nonuser_event

fn handle_nonuser_event(&self, wrapper: EventWrapper) {
        // Dialog Still works
        rfd::open();

        if let Some(ref mut callback) = *self.callback.lock().unwrap() {
            // Nope after the lock this one is frozen
            rfd::open();

            match wrapper {
                EventWrapper::StaticEvent(event) => {
                    callback.handle_nonuser_event(event, &mut *self.control_flow.lock().unwrap())
                }
                EventWrapper::EventProxy(proxy) => self.handle_proxy(proxy, callback),
            }
        }

        // After the unlock we are back in business, works as expected
        rfd::open();
}

EDIT: I belive I understand whats heappending now.

// 1. handle_nonuser_event() called for the first time.
StaticEvent(
    NewEvents(
        Init,
    ),
)

// 2. I'm opening a dialog inside of event loop callback

// 3. handle_nonuser_event() is called for the second time.
// heapens exacly when dialogs shows up
// I'm preaty sure that this event is caused by dialog
StaticEvent(
    NewEvents(
        Poll,
    ),
)

// 4. We have a frozzen program, Pool Event is waiting for unlock, but unlock will never heappen

EDIT2:

  • Or maybe it's my lack of Mac knowledge, is winit event loop somehow tied to system dialogs?
    Yeha it definetly is tied, It retrives all mouse events from system dialogs, and it can eassly cause unexpected locks on mutexes

EDIT3: Just in case that my explanation sucks, here is a diagram of what I think is happening:
img

@PolyMeilex
Copy link
Contributor Author

@vbogaevsky
Ignoring all incoming events while the callback is running solves the issue (this makes sure that no locks are made to control_flow mutex, and calback_mutex), but I don't know the full impact of this change, so I would like to hear the opinion of someone with more knowledge about macOS backend.
And of course, ignoring some events is always something that should be taken with care.

Example implementation of this change: PolyMeilex@69a6129

@chrisduerr
As I know that vbogaevsky is kinda inactive lately, I would like to ask you how should I proceed with this? Ignoring events sounds kinda scary (that's why I'm writing here instead of making a PR), but I'm out of ideas.

@adamnemecek
Copy link

Any update on this @chrisduerr ?

@thenlevy
Copy link

I am also having this problem. Opening a file dialog with either native-dialog-rs0.5.2 or nfd20.2.3 causes the file dialog to freeze on macos.
Using PolyMeilex's fork of winit solves the issue for me.

Also, native-dialog-rs 0.4.* works fine even with the crates.io version of winit. If I'm not mistaking, this previous version used something called AppleScript, which has been replaced by something called cocoa (I don't know much about macos programming so this could be wrong/irrelevant..). Here is the discussion about this change in native-dialog-rs.

@PolyMeilex
Copy link
Contributor Author

PolyMeilex commented Jan 12, 2021

Both nfd2 and my rfd use objc API, AppleScript is not used as it is too limiting.

@PolyMeilex
Copy link
Contributor Author

I've implemented async/await syntax in rfd as a workaround to the problem, so if anyone is interested feel free to give it a try. It can be used with the crates.io version of winit (example).

Issue stays open, as the problem still exists inside winit and should be fixed nonetheless.

@vihdzp
Copy link

vihdzp commented May 17, 2021

I've been using PolyMeilex's fork of winit, and it's completely solved this issue for me. What's stopping it from being merged into the main branch?

@maroider
Copy link
Member

maroider commented May 17, 2021

What's stopping it from being merged into the main branch?

  1. There simply isn't a PR open for that (as far as I can tell).
  2. @ArturKovacs may not be aware of this issue.

@PolyMeilex
Copy link
Contributor Author

I'm using my fork in my neothesia project, and I haven't heard any complaints yet.
But as I said:
I don't know the full impact of this change, so I would like to hear the opinion of someone with more knowledge about macOS backend.
I'm essentially ignoring all incoming events in some cases, so I don't like this solution at all, and I have 0 knowledge about macOS windowing.

@ArturKovacs
Copy link
Contributor

Oh so I guess this was the issue you mentioned to me @adamnemecek. I think that the lock function call on the callback should be replaced with a try_lock, since the callback must only be called from the main thread and therefore finding the mutex locked at any point must mean that there was re-entrance (which would cause deadlock anyways) or must mean that there was a developer error and the callback is being accessed from a non-main thread which it shouldn't be.

I just wanted to write this down but unfortunately I don't have time look into this right now, but I will do hopefully soon.

This whole re-entrance and locking thing might actually be the reason why we queue events instead of dispatching them synchronously, but I'm not sure and that's mostly related to #1911.

emilk added a commit to emilk/egui that referenced this issue Aug 19, 2021
Previously app code was run from within the event loop
which lead to file dialogs (e.g. using nfd2) to hang
(see rust-windowing/winit#1779)

Now egui_glium polls for events and then runs the app code.
@emilk
Copy link
Contributor

emilk commented Aug 19, 2021

I managed to fix this by switching from event_loop.run(|…| { … file_dialog(); }) to loop { event_loop.run_return(…); … file_dialog(); }:

emilk/egui#631

emilk added a commit to emilk/egui that referenced this issue Aug 20, 2021
Previously app code was run from within the event loop
which lead to file dialogs (e.g. using nfd2) to hang
(see rust-windowing/winit#1779)

Now egui_glium polls for events and then runs the app code.
mankinskin pushed a commit to mankinskin/egui that referenced this issue Sep 29, 2021
…k#631)

Previously app code was run from within the event loop
which lead to file dialogs (e.g. using nfd2) to hang
(see rust-windowing/winit#1779)

Now egui_glium polls for events and then runs the app code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

8 participants