[macOS] Fixes Crash of cxx destruction when App will terminate

[zhongwuzw]

Fixes flutter/flutter#90783 (comment).

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• [] I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
  writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[jmagman]

This seems like a good change, when the app is terminated it won't dealloc the engine, which is the only place it's currently explicitly shut down.

@stuartmorgan do you see any gotchas here? (@chunhtai is OOO)

[stuartmorgan]

The obvious downside is that this will slow down quitting the app. Better slow than crashing, but it's unusual to have to do explicit object teardown in the exit flow.

I'm trying to understand from the discussion in the other PR why it's necessary. @chinmaygarde does the engine violate the C++ style guide's prohibition on global objects with non-trivial destructors?

[chinmaygarde]

does the engine violate the C++ style guide's prohibition on global objects with non-trivial destructors?

Oh yeah. All over the place. There used to be an old Clang plugin built on libTooling that could detect such objects in the static storage duration. We had trouble getting it working on the platforms Flutter needs to build for. Maybe there is a built in warning for that now. I am not sure. In any case, there should only be a few cases in the engine that we can fixup. I am not sure about our dependencies though.

The obvious downside is that this will slow down quitting the app.

This isn't anything new though as the non-trivial destructors are still being called. Its just that they are crashy today because of unstable call order.

[jmagman]

Forgot to formally LGTM

[stuartmorgan]

The obvious downside is that this will slow down quitting the app.

This isn't anything new though as the non-trivial destructors are still being called. Its just that they are crashy today because of unstable call order.

Well, presumably it'll do more now. But mostly I meant that ideally the fix would be to not have the violations in the first place, rather than to cater to them with more explicit tear-down.

[zhongwuzw]

Maybe related contexts of similar crash: React Native and ComponentKit