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

ignore the window arc on macos #13221

Closed

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented May 3, 2024

Objective

Solution

  • Ignore the window arc. The original issue didn't happen on macOS anyway

@mockersf mockersf added A-Windowing Platform-agnostic interface layer to run your app in O-MacOS Specific to the MacOS (Apple) desktop operating system labels May 3, 2024
@mockersf mockersf added this to the 0.14 milestone May 3, 2024
@mockersf mockersf requested a review from rparrett May 3, 2024 22:54
@ids1024
Copy link

ids1024 commented May 3, 2024

So with this change, there's a RawHandleWrapper for a window that no longer exists?

That means the RawWindowHandle contains a dangling pointer, which is definitely an issue if its actually used anywhere after that.

Not necessarily bad for a temporarily workaround (given this issue already existed) but definitely warrants more investigation.

@alice-i-cecile alice-i-cecile added X-Contentious There are nontrivial implications that should be thought through D-Straightforward Simple bug fixes and API improvements, docs, test and examples labels May 4, 2024
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't hate this as a workaround. Messy though, and I'd obviously prefer a real fix.

@rparrett
Copy link
Contributor

rparrett commented May 4, 2024

I can confirm that this fixes the issue, but I have not been following along with the relevant changes.

I am seeing this error spam when closing windows now, however:

2024-05-04T01:24:08.745268Z  INFO bevy_winit::system: Creating new window "App" (Entity { index: 0, generation: 1 })
2024-05-04T01:24:10.548038Z  INFO bevy_window::system: No windows are open, exiting
2024-05-04T01:24:10.549105Z  INFO bevy_winit::system: Closing window Entity { index: 0, generation: 1 }
2024-05-04T01:24:10.566197Z  INFO bevy_window::system: No windows are open, exiting
2024-05-04T01:24:10.567983Z ERROR bevy_winit: Failed to send a app exit notification! This is a bug. Reason: sending on a full channel
2024-05-04T01:24:10.568591Z ERROR bevy_winit: Failed to send a app exit notification! This is a bug. Reason: sending on a full channel
2024-05-04T01:24:10.568675Z ERROR bevy_winit: Failed to send a app exit notification! This is a bug. Reason: sending on a full channel

The errors go away if I revert 15687b5.

@alice-i-cecile
Copy link
Member

#13203 should fix the warning spam: that's a cross platform problem that's not particularly related.

@ids1024
Copy link

ids1024 commented May 4, 2024

I've done a bit more investigation of the issue here, with some comments about it on #13208.

It looks like the issue that the winit::Window is dropped on a non-main thread. On macOS, this executes on the main thread and blocks on completion of that. But it seems something is running on the main thread and blocking it, so winit::EventLoop/CFRunLoop can't make progress and actually run that?

Not sure exactly have bevy is managing what the main thread is doing here, but this sounds non-trivial to fix. So this workaround may be needed for now.

@alice-i-cecile
Copy link
Member

"Resources that must be dropped on the main thread" is exactly what NonSend data is for.

@mockersf
Copy link
Member Author

mockersf commented May 4, 2024

This isn't a resource, it's a component... so it would mean we can't put the window in a component

@ids1024
Copy link

ids1024 commented May 4, 2024

Dropping the winit::Window from off the main thread isn't a problem in itself, but it is a problem if something is blocking the winit::EventLoop. Then it deadlocks. (This is also true if another thread tried to call other winit methods.)`

@mockersf
Copy link
Member Author

mockersf commented May 5, 2024

closing in favour of #13236

@mockersf mockersf closed this May 5, 2024
@mockersf mockersf deleted the tmp-fix-macos-not-stopping branch May 17, 2024 21:15
@mockersf mockersf restored the tmp-fix-macos-not-stopping branch May 17, 2024 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Windowing Platform-agnostic interface layer to run your app in D-Straightforward Simple bug fixes and API improvements, docs, test and examples O-MacOS Specific to the MacOS (Apple) desktop operating system X-Contentious There are nontrivial implications that should be thought through
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants