Skip to content

When a plugin throws during startup, cleanup #17

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.

Sign up for GitHub

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

Merged
wanderingbort merged 1 commit into from
Mar 30, 2018
Merged

When a plugin throws during startup, cleanup #17

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions application.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,13 @@ bfs::path application::get_logging_conf() const {
}

void application::startup() {
for (auto plugin : initialized_plugins)
plugin->startup();
try {
for (auto plugin : initialized_plugins)
plugin->startup();
} catch(...) {
Copy link

@wanderingbort wanderingbort Mar 29, 2018

Choose a reason for hiding this comment

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

considering a case where the exception thrown from startup but leaves plugins in a state where shutdown is not possible and therefore throws itself, should we catch log and drop any exception thrown from shutdown so that the root cause (the startup exception) is the one that bubbles out?

Copy link
Author

@spoonincode spoonincode Mar 30, 2018

Choose a reason for hiding this comment

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

I don't disagree but this simple "surgical" fix removes an easy high visibility crash. I would rather take this fix now and then add an issue for that more complex case if truly desired. BUT... You are rigth though, there are some interesting cases here where a plugin could throw half way through, thus not having its shutdown() called, but still its dtor.

And now that I look closer it appears that net_plugin might be the only violator who throws something uncaught on startup. So I suppose there is an argument that net_plugin should be fixed.

shutdown();
throw;
}
}

application& application::instance() {
Expand Down