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

(#3729): when an exception is thrown during render, we no longer crash the compositor #3750

Closed
wants to merge 1 commit into from

Conversation

mattkae
Copy link
Contributor

@mattkae mattkae commented Feb 10, 2025

fixes #3729

What's new?

@mattkae mattkae marked this pull request as ready for review February 10, 2025 14:54
@mattkae mattkae requested a review from a team as a code owner February 10, 2025 14:54
Copy link
Collaborator

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

I still have to think about whether this is the best way to handle the problem. It isn't entirely clear cut.

But, from a quick reading, we have a logging function to report exceptions and we should use it. (not just exception::what())

Comment on lines 128 to 130
// An exception may be thrown by the underlying platform during render.
// While bad, we do not want this to crash the compositor entirely.
mir::log_error("Encountered exception while rendering: %s", e.what());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// An exception may be thrown by the underlying platform during render.
// While bad, we do not want this to crash the compositor entirely.
mir::log_error("Encountered exception while rendering: %s", e.what());
// An exception may be thrown by the underlying platform during render.
// While bad, we do not want this to crash the compositor entirely.
log(logging::Severity::error, MIR_LOG_COMPONENT, std::current_exception(), "Encountered exception while rendering");

@mattkae
Copy link
Contributor Author

mattkae commented Feb 10, 2025

I still have to think about whether this is the best way to handle the problem. It isn't entirely clear cut.

But, from a quick reading, we have a logging function to report exceptions and we should use it. (not just exception::what())

I don't love it either, which is why I made the other ticket to address the main underlying problem. If we would rather tackle that than eating the exception, I think that would be fair. Although I wouldn't necessarily consider a failure to render on a single output to be "fatal" per se, which is why I think the answer might be both.

@mattkae mattkae closed this Feb 11, 2025
@mattkae mattkae deleted the alternative/3729 branch February 11, 2025 14:26
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

Successfully merging this pull request may close these issues.

If I time my output unplug at the right moment, then I can cause Mir to crash (tested on current main)
2 participants