main: use custom main common to disable listening for termination signals

[junr03]

Description: Disabling signal handling in the Server::Options makes it so that the server's event dispatcher does not listen for termination signals such as SIGTERM, SIGINT, etc. Previous crashes in iOS were experienced due to out-of-band event loop exit as described in #831. Ignoring termination signals makes it more likely that the event loop will only exit due to Engine destruction.

This PR introduces Envoy::MobileMainCommon, as this is the canonical way to customize how main runs, and options setup per envoyproxy/envoy#3424. The new Envoy::MobileMainCommon also does away with other logic in Envoy::MainCommon that does not apply to Envoy Mobile.

Risk Level: med - low-level change in termination handling
Testing: unit test to assert option is correctly set. End to end test with iOS and Android devices to ensure clean exit when the app using envoy mobile exits (and thus destructs the engine). Moreover, there is no event loop exit any longer when the simulator app receives a SIGTERM, i.e., the event dispatcher is no longer listening to SIGTERM and exiting due to them.

Potentially fixes #831. Will need to verify with wider client release.

Signed-off-by: Jose Nino jnino@lyft.com

[junr03]

@mattklein123, thanks for the ideas and pointers in #831. Based on my research online, there is a good chance that making it so that the event loop does not listen for termination signals will prevent exits outside of the Engine destructor. @Reflejo if you have some time, I'd appreciate your eyes here. Your expertise would definitely help verify my research.

[mattklein123]

Nice!

[Reflejo]

This looks good for the attempt you are trying to make. I'm still not convinced this is due to a signal by reading the other issue but if it is:

Signal handling is hard. I think it's entirely possible for that null pointer bug to be a red herring (ie. the app crashed for a different reason, sent a SIGTERM / SIGINT so things get shut down twice and the initial crash is lost). Bugsnag is trying to catch signals as well and the ordering is unpredictable.

I don't think there are many other cases where iOS will send anything other than SIGKILL but maybe things changed on recent OSes.

Can we send ENVOY_LOG()s to bugsnag as breadcrumbs? I think that'd be helpful. Or at least have a way to consume logs on crashes.

[junr03]

I think it's entirely possible for that null pointer bug to be a red herring (ie. the app crashed for a different reason, sent a SIGTERM / SIGINT.

I see, potentially. The scenario you described reinforces my wanting to make this change anyways, because the envoy event dispatcher should not be listening to termination signals, and thus leading to envoy's thread termination. The way we organized shutdown the only way to maintain predictable behavior is to have the event loop exit from the exit() call issued in the Engine's destructor. For that reason, this change should still be done to prevent "out of band" event loop exits.

Can we send ENVOY_LOG()s to bugsnag as breadcrumbs? I think that'd be helpful. Or at least have a way to consume logs on crashes.

Yeah, I have thought about that before (doc), but we have not gotten around to prioritize that work.