Skip to content

Expose OS.disable_crash_handler() for extensions #108268

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

limbonaut
Copy link
Contributor

Allow disabling crash handler from GDExtension API.

We’ve stumbled upon a bit of a problem in Sentry SDK. It seems that Linux builds aren’t shutting down properly when they crash. The process gets stuck running indefinitely after the crash. This PR would allow us to unregister Godot's crash handler and use ours instead.

We'd really appreciate if this could be added to version 4.5. It can be a real pain in some scenarios, like dedicated servers not automatically restarted because the process gets stuck. It's a relatively simple change.

@timothyqiu
Copy link
Member

I think exposing a method is an overkill. It could be a project setting debug/settings/crash_handler/enabled instead.

This is also how accessibility related functions are disabled.

You can run your application with the --disable-crash-handler command line option as a workaround if you are in a hurry.

@limbonaut
Copy link
Contributor Author

I was considering a project setting approach and I think it has substantial drawbacks. We wouldn't be able to automate this process during launch, because by the time we can change this setting, it's already too late. We'd have to prompt users to change this setting and it will be subject to human error. The other downside, is that it would have to be two settings, one for the exported project and another one for the editor because we also support running our extension within the editor as well. In my opinion, the least disruptive change would be to add an API method. Let me know what you think!

@timothyqiu
Copy link
Member

Yeah, I agree exposing disable_crash_handler() is a simpler approach.


Beside that, I think there are several things mixed up here:

  • Linux builds running indefinitely after a crash
    • Would be better to submit a bug report. AFAIK, it takes a long time for the default crash handler to generate crash messages. However, they shouldn't run indefinitely.
  • Dedicated servers don't restart automatically
    • In addition to the bugs mentioned above, run the server with the --disable-crash-handler option.
  • Disable the default crash handler in the exported game
    • If using the project settings approach, your extension can modify the project settings when in the editor.
  • Disable the default crash handler in the editor
    • It does not require an extra project setting AFAIK.
    • I can understand the use case. But it sounds strange that a user using your extension would make your extension collect all editor crashes. What if two different addons competing to do so?

@limbonaut
Copy link
Contributor Author

Linux builds running indefinitely after a crash

The problem is that defining multiple crash handlers is poorly supported across frameworks and may cause issues that are hard to deal with. You can read more about it here. There are frameworks that allow chaining crash handlers, but it requires explicit coordination betwene them, usually by chaining inside a single registered handler.

In addition to the bugs mentioned above, run the server with the --disable-crash-handler option.

I think putting this on users shoulders is not great UX. It's more of a workaround. We're making extension that helps debugging shipped games, and having things working out-of-the-box is highly desirable.

your extension can modify the project settings when in the editor

We can do that, and I'm not against it. I submitted this version, because I think the API method is a slightly better approach.

But it sounds strange that a user using your extension would make your extension collect all editor crashes.

It's not enabled by default. This is reserved for specialized use-cases where the editor is the product (forks that want to ship with the extension).

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

Successfully merging this pull request may close these issues.

3 participants