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

Fix for WFGP-284, Embedded server execution should be isolated in its own classloader #301

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

jfdenise
Copy link
Collaborator

Isolated execution of Embedded Server during provisioning.

  • Config generation is operated from 2 classloaders. First one load the config generator + CLI (DMR, ...). This one delegates to the context classloader. That is OK, no embedded server part.
  • The second one only load the embedded server + all its dependencies (such as DMR all from cli:client) + JBoss Modules

So DMR is loaded twice. In the first classloader, converted to yaml String to be consumed on the embedded side.

This isolation should be even more strict than the forked process approach.

Copy link
Member

@jamezp jamezp left a comment

Choose a reason for hiding this comment

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

In general this looks okay. I'm not sure I feel about the odd cases for variables, but that's just a personal preference :) I also understand it makes it look more like real calls so I do get that part.

I do wonder a bit if we should be using MethodHandle's instead of reflection. Likely not a huge deal, but there may be some performance impact. It might not be enough to worry about though.

@jfdenise jfdenise changed the title Isolated embedded server execution Fix for WFGP-284, Embedded server execution should be isolated in its own classloader Jul 25, 2024
@jfdenise jfdenise force-pushed the isolate-cl branch 2 times, most recently from e402610 to ddcaa1e Compare July 26, 2024 09:23
@jfdenise jfdenise merged commit fb962b3 into wildfly:main Jul 26, 2024
16 of 18 checks passed
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