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

app.quit() causes panics #743

Open
ErikNatanael opened this issue Jun 1, 2021 · 3 comments
Open

app.quit() causes panics #743

ErikNatanael opened this issue Jun 1, 2021 · 3 comments
Labels

Comments

@ErikNatanael
Copy link
Contributor

ErikNatanael commented Jun 1, 2021

Using the approach in the draw_capture_hi_res example of waiting for captured frames doesn't work if you want to manually quite the program at some point. quit seems to only destroy the windows, but leaves the run loop running which depends on there being a window, as does the exit callback in question:
https://github.com/nannou-org/nannou/blob/master/examples/draw/draw_capture_hi_res.rs#L190

Exiting with the ESCAPE key does things in the opposite order:

  1. Run the exit callback
  2. Return from the run_loop
  3. Cleanup?
    Reference: https://github.com/nannou-org/nannou/blob/master/nannou/src/app.rs#L1321

Waiting manually by calling the code in the exit callback and then calling app.quit() sometimes works and sometimes panics.

thread 'main' panicked at 'no window for redraw request ID', /home/USERNAME/.cargo/registry/src/github.com-1ecc6299db9ec823/nannou-0.16.0/src/app.rs:761:14

It would be nice if the behaviour of app.quit() was consistent with hitting the ESCAPE key e.g. by setting a flag that is checked in the run_loop.

@mitchmindtree
Copy link
Member

Thanks for opening this @ErikNatanael !

I remember when #712 landed thinking that it was a clever hack to add the quit functionality without needing any extra flags or logic, however I think needing access to windows in exit is a great case against this approach.

quit seems to only destroy the windows, but leaves the run loop running which depends on there being a window, as does the exit callback in question:

Just to clarify, your issue is specifically with the exit callback right? I don't think any other event loop callbacks should be called after app.quit() is called (though if called in an update, it's possible the following view is called before exiting - can't quite remember exactly).

I think the proper fix is likely to not clear all windows on app.quit() and instead set some dedicated flag that can be checked here so that windows are still available when exit is called and are only cleaned up when drop runs afterwards. Does it sound like a solution like this might work for your case?

@ErikNatanael
Copy link
Contributor Author

Thanks for having a look @mitchmintree !

quit seems to only destroy the windows, but leaves the run loop running which depends on there being a window, as does the exit callback in question:

Just to clarify, your issue is specifically with the exit callback right? I don't think any other event loop callbacks should be called after app.quit() is called (though if called in an update, it's possible the following view is called before exiting - can't quite remember exactly).

Ah yes, I should clarify. The exit callback not being called is the main issue, but in testing workarounds I ran into panics when calling app.quit() in certain parts of my update() function. I haven't been able to pinpoint why exactly, it's a strange issue where app.quit() cannot be called first or last in the update() function in my specific case, but at some points in the middle it works without panicking. It could have something to do with me drawing everything to a separate texture like in the draw_capture_hi_res example, and there being some hidden internal reference to winit or wgpu stuff on other threads at certain points? Just guessing!

I think the proper fix is likely to not clear all windows on app.quit() and instead set some dedicated flag that can be checked here so that windows are still available when exit is called and are only cleaned up when drop runs afterwards. Does it sound like a solution like this might work for your case?

Sounds like a great solution from my point of view! Having just one path to exiting seems more easily maintainable as well.

@ErikNatanael
Copy link
Contributor Author

Just realised the panic I had with app.quit() in the update function was probably because I had the window from app.main_window() borrowed when running app.quit().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants