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

Bugfix: Use-after-free when same plugin is loaded twice #343

Merged
merged 9 commits into from
Aug 19, 2024

Conversation

nathanwbrei
Copy link
Collaborator

@nathanwbrei nathanwbrei commented Aug 19, 2024

This bugfix addresses issue #333.
It introduces some behavior changes: (!)

  • If the user adds duplicate JFactories to the same factory set, JANA will throw an exception rather than silently destroying the second copy. This is necessary because otherwise JANA can't enforce the integrity of JMultifactories.
  • If the user includes the same plugin twice in the plugins parameter, it only gets loaded once.
  • The user-provided plugins parameter no longer clobbers the projects' default plugins specified via JApplication::AddPlugin. Rather, the project's default plugins are loaded first, and any additional user plugins specified by the plugins parameter are loaded afterwards.
  • JPluginLoader now throws an exception if the user provides a plugin name that is really a path. This is because the logic for ensuring that plugins get loaded exactly once becomes extremely hairy when we account for plugin name conflicts that are off the search path. Consider the example jana -Pplugins=MyPlugin,other/plugin/dir/MyPlugin.so -Pjana:plugin_path=normal/plugin/dir where MyPlugin.so can be found in both places. We aren't doing the users any favors by letting them get into this mess, and we are better off with a plugin loader that is easier to reason about.

@nathanwbrei nathanwbrei merged commit ef43454 into master Aug 19, 2024
9 checks passed
@nathanwbrei nathanwbrei deleted the nbrei_issue333 branch August 19, 2024 17:28
@veprbl
Copy link
Contributor

veprbl commented Aug 22, 2024

Massive work. Big thanks!

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