Panda load plugin enhancements #1371
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MarkMankins
force-pushed
the
panda_load_plugin_enhancements
branch
from
4e6dc6c to
6a1c4ff
Compare
Moved the logic to dlopen libpanda into seperate utility functions. This makes it easier to understand what _panda_load_plugin is doing.
Use the existing panda_plugins array to determine if a plugin has already been loaded.
Added a new boolean field to panda_plugins structure to track when a request to unload a plugin has been made.
Combine similar functions panda_require_from_library and panda_require into a new utility.
Not a public API.
if magic symbol PANDA_EXPORT_SYMBOLS_plugin_name is present in the plugin being loaded.
Added an empty check for filename too.
This was probably never going to cause a problem, but the code assumed a plugin was never greater than 256 bytes, but allowed for that case to occur, which would cause things to break if it ever happened.
For help output, always print to stdout
Previously, running panda-system-i386 -panda osi_linux:help=y
would output that osi failed to load because PANDA_EXPORT_SYMBOLS_osi_linux
was not defined. The error message output had nothing to do with
why osi_linux failed to load. In this case, osi_linux doesn't
panda_require('osi') (which may be a bug) - so when it calls init_osi_api
things go sideways. The error message was being obtained in init_osi_api
from calling dlerror() - which was reporting the missing symbol, which
isn't an error at all as the symbol is optional and not expected to be
found when loading most plugins.
MarkMankins
force-pushed
the
panda_load_plugin_enhancements
branch
from
6a1c4ff to
5f0c31b
Compare
|
Rebased to latest dev. |
|
Thanks, these seem like some great changes - I had been hoping to get more eyes on the PR before merging, but I'm sure someone will let us know if this breaks anything. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The major new functionality here is in do_check_export_symbols().
I have a rather large C++ panda plugin that I'm trying to split up into separate plugins. I would like to have a base C++ class in plugin plugin_x. I would like to have plugins plugin_y and plugin_z extend my C++ class defined in plugin_x to implement custom functionality.
This doesn't work with the current logic for loading plugins, as each plugin loaded doesn't provide symbols to subsequently loaded plugins.
do_check_export_symbols changes that behavior, but only for plugins that opt-in to the new behavior.
In plugin_x, by defining a magic symbol:
panda_plugin_x.so will be opened (via dlopen) with the RTLD_GLOBAL flag. This makes symbols defined in plugin_x available to subsequently loaded plugins.
The majority of the commits in this PR aren't related to this new function. I tried to clean up the various functions that deal with plugin loading and unloading, mostly in callbacks.c. It's possible I broke something here that someone was using - I can rework things if we find I did break something.
Major changes (including potential breaking changes in obscure cases):