hal icon indicating copy to clipboard operation
hal copied to clipboard

plugin_manager.cpp crashes while loading already unloaded plugins

Open FabianAlbertRub opened this issue 5 years ago • 4 comments

Describe the bug If we load a plugin, unload it and load it later again, the plugin_manager crashes with a segmentation fault in src/core/plugin_manager.cpp l. 163

To Reproduce Steps to reproduce the behavior: for example:

plugin_manager::load_all_plugins();
plugin_manager::unload_all_plugins();
plugin_manager::load_all_plugins(); // <- crashes with SIGSEGV

Expected behavior It should not crash I guess ;P

Additional Information The crash occurs in line 163 of src/core/plugin_manager.cpp:

...
std::set<std::string> dep_file_name = factory->get_dependencies();
...

I think the reason for the problem is, that the unload function deletes the static factory object of the corresponding gate library in line 245 (delete factory). Since the object is defined as static, it will not be recreated in a second call of the get_factory() function. Of course, for the main usage you would not unload and load a library after, but in the context of testing it is necessary for me to have such a functionality ;)

FabianAlbertRub avatar Jun 21 '19 18:06 FabianAlbertRub

Issue-Label Bot is automatically applying the label Type: Bug to this issue, with a confidence of 0.98. Please mark this comment with :thumbsup: or :thumbsdown: to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

issue-label-bot[bot] avatar Jun 21 '19 18:06 issue-label-bot[bot]

@FabianAlbertRub can you please check if this issue persists on any build after commit 47953bfea4fba58cb51dd534dc833cc9916206c5 "plugin system rewrite"? I currently can't reproduce the issue.

RenWal avatar Jan 08 '20 18:01 RenWal

You are right, the plugin manager does not crash anymore. However, it can't reload plugins with cli-options, since they are considered to be already used by another plugin. In have just updated my tests for the plugin manager in the branch "tests" (parallel to master), where you can see the issue I described.

FabianAlbertRub avatar Jan 09 '20 15:01 FabianAlbertRub

@RenWal @FabianAlbertRub any update here? iirc this is already fixed since a few months

not-a-trojan avatar Nov 30 '20 17:11 not-a-trojan