Skip to content

When a plugin throws during startup, cleanup#17

Merged
wanderingbort merged 1 commit intomasterfrom
cleanup_on_startup_throw
Mar 30, 2018
Merged

When a plugin throws during startup, cleanup#17
wanderingbort merged 1 commit intomasterfrom
cleanup_on_startup_throw

Conversation

@spoonincode
Copy link

Some of our plugins will throw an exception on startup. For example, the http or net plugin will throw if it can’t bind to an address. Exceptions thrown at this stage are uncaught and cause the process to exit.

However, in the process of unwinding the stack to exit, application’s dtor will call all of the already started plugins dtors. Some of these plugins don’t handle the case where their dtor is called without shtudown() being called on the plugin first. Result can be a (benign, but still embarrassing) crash on exit.

Change appbase so that any exception thrown during startup() is caught, then all plugins shutdown, then exception is rethrown so that the app continues to exit.

Some of our plugins will throw an exception on startup. For example, the http or net plugin will throw if it can’t bind to an address. Exceptions thrown at this stage are uncaught and cause the process to exit.

However, in the process of unwinding the stack to exit, application’s dtor will call all of the already started plugins dtors. Some of these plugins don’t handle the case where their dtor is called without shtudown() being called on the plugin first. Result can be a (benign, but still embarrassing) crash on exit.

Change appbase so that any exception thrown during startup() is caught, then all plugins shutdown, then exception is rethrown so that the app continues to exit.
try {
for (auto plugin : initialized_plugins)
plugin->startup();
} catch(...) {

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

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.

@wanderingbort wanderingbort merged commit 3bfd8b5 into master Mar 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants