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

Add main function wrapper to catch exceptions and avoid annoying error popup on windows #768

Merged
merged 11 commits into from
May 16, 2020

Conversation

res2k
Copy link
Contributor

@res2k res2k commented Apr 17, 2020

Description

This adds a wrapper around main() functions to deal with unhandled exceptions by printing a "fatal" log message and exiting.
The motivation is the behavior of unhandled exceptions on MSVC. There, such an exception simply terminates the program, but the exception message is not output anywhere! This can make troubleshooting very confusing, as the log of a tool gives no indication of a problem.

Features list

Not really a feature.

Implementation remarks

Wrapper is "opt-in" by including system/main.hpp.
The header defines a macro main so the main() function in the source appears like the real one even though it's wrapped. It's also convenient.

@fabiencastan
Copy link
Member

Thanks for the contribution. I fully agree with the need!

Few remarks regarding the implementation:
1/ I would prefer to find a solution without having to create a define on "main". You can easily have a library with a member variable named "int main;" or something else.
We could just manually rename "main" by "aliceVision_main" in all main_*.cpp files so it is explicit and if the main.hpp is not included, the compilation will fail.

2/ I would keep main.hpp as a header only file and I would also remove the intermediate aliceVision_main_wrapper, as we can just put the try/catch in the real main.

What do think?

@res2k
Copy link
Contributor Author

res2k commented Apr 18, 2020

Hi,
I've made the changes you requested.

@simogasp simogasp added this to the 2020.1.0 milestone Apr 19, 2020
*/
int aliceVision_main(int argc, char* argv[]);

// Inline to ensure the right main() is used (libf2c also has one...)
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest a more explicit comment, like:

// Implementation of the unique main entry point.
// This method will call aliceVision_main and catch the exceptions to
// log the error message and avoid seg fault on exception on windows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I expanded the comment.

@fabiencastan
Copy link
Member

It would be good to apply it on all main_*.cpp and not only software/pipeline.

@res2k
Copy link
Contributor Author

res2k commented Apr 19, 2020

It would be good to apply it on all main_*.cpp and not only software/pipeline.

Yes. I would have done that as a follow-up PR, but I can add it to this one as well.

@fabiencastan fabiencastan merged commit 9ba4860 into alicevision:develop May 16, 2020
@fabiencastan fabiencastan changed the title main() wrapper Add main function wrapper to catch exceptions and avoid annoying error popup on windows Aug 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants