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

GEODE-8705: Introduce classloader isolation for deployments using JBoss Modules. #7348

Closed
wants to merge 21 commits into from

Conversation

yozaner1324
Copy link
Contributor

@yozaner1324 yozaner1324 commented Feb 7, 2022

  • introduce JarDeploymentService and ClassPathService
    * introduce Gradle plugin to generate module descriptors
    * add --enable_classloader_isolation flag to start server command
    * refactored DUnit tests to optionally run with classloader isolation
    * add checks to CI to run DUnits with classloader isolation
    * updated docs to reflect new Classloader isolation

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?

  • Has your PR been rebased against the latest commit within the target branch (typically develop)?

  • Is your initial contribution a single, squashed commit?

  • Does gradlew build run cleanly?

  • Have you written or updated unit tests to verify your changes?

  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

@lgtm-com
Copy link

lgtm-com bot commented Feb 8, 2022

This pull request introduces 8 alerts when merging fa9f836 into e7094ee - view on LGTM.com

new alerts:

  • 3 for Potential input resource leak
  • 2 for Container contents are never accessed
  • 2 for Dereferenced variable may be null
  • 1 for Use of a broken or risky cryptographic algorithm

@yozaner1324 yozaner1324 force-pushed the GEODE-8705 branch 2 times, most recently from 7316c22 to a313458 Compare February 8, 2022 18:17
@lgtm-com
Copy link

lgtm-com bot commented Feb 8, 2022

This pull request introduces 8 alerts when merging a313458 into 551d7d7 - view on LGTM.com

new alerts:

  • 3 for Potential input resource leak
  • 2 for Container contents are never accessed
  • 2 for Dereferenced variable may be null
  • 1 for Use of a broken or risky cryptographic algorithm

@lgtm-com
Copy link

lgtm-com bot commented Feb 8, 2022

This pull request introduces 8 alerts when merging aa21293 into ce57e9f - view on LGTM.com

new alerts:

  • 3 for Potential input resource leak
  • 2 for Container contents are never accessed
  • 2 for Dereferenced variable may be null
  • 1 for Use of a broken or risky cryptographic algorithm

@lgtm-com
Copy link

lgtm-com bot commented Feb 10, 2022

This pull request introduces 8 alerts when merging 6736207 into fbe4e66 - view on LGTM.com

new alerts:

  • 3 for Potential input resource leak
  • 2 for Container contents are never accessed
  • 2 for Dereferenced variable may be null
  • 1 for Use of a broken or risky cryptographic algorithm

@lgtm-com
Copy link

lgtm-com bot commented Feb 11, 2022

This pull request introduces 8 alerts when merging b3e3359 into 8825d95 - view on LGTM.com

new alerts:

  • 3 for Potential input resource leak
  • 2 for Container contents are never accessed
  • 2 for Dereferenced variable may be null
  • 1 for Use of a broken or risky cryptographic algorithm

@lgtm-com
Copy link

lgtm-com bot commented Feb 11, 2022

This pull request introduces 8 alerts when merging 6dc6b1c into 21215d8 - view on LGTM.com

new alerts:

  • 3 for Potential input resource leak
  • 2 for Container contents are never accessed
  • 2 for Dereferenced variable may be null
  • 1 for Use of a broken or risky cryptographic algorithm

@lgtm-com
Copy link

lgtm-com bot commented Feb 11, 2022

This pull request introduces 8 alerts when merging 17b1f18 into 9fa1c1b - view on LGTM.com

new alerts:

  • 3 for Potential input resource leak
  • 2 for Container contents are never accessed
  • 2 for Dereferenced variable may be null
  • 1 for Use of a broken or risky cryptographic algorithm

@yozaner1324 yozaner1324 force-pushed the GEODE-8705 branch 2 times, most recently from 66823ad to 27f0f31 Compare February 11, 2022 22:44
@lgtm-com
Copy link

lgtm-com bot commented Feb 11, 2022

This pull request introduces 6 alerts when merging 27f0f31 into 9fa1c1b - view on LGTM.com

new alerts:

  • 3 for Potential input resource leak
  • 2 for Dereferenced variable may be null
  • 1 for Use of a broken or risky cryptographic algorithm

@lgtm-com
Copy link

lgtm-com bot commented Feb 14, 2022

This pull request introduces 8 alerts when merging 0d70aeb into a98197b - view on LGTM.com

new alerts:

  • 3 for Potential input resource leak
  • 2 for Container contents are never accessed
  • 2 for Dereferenced variable may be null
  • 1 for Use of a broken or risky cryptographic algorithm

@kohlmu-pivotal kohlmu-pivotal added jdk17 add this label to get Linux JDK17 PR checks too windows-jdk17 add this label to get Windows JDK17 PR checks too windows-jdk8 add this label to get Windows JDK8 PR checks too labels Jun 16, 2022
@kohlmu-pivotal kohlmu-pivotal removed jdk17 add this label to get Linux JDK17 PR checks too windows-jdk17 add this label to get Windows JDK17 PR checks too labels Jun 16, 2022
yozaner1324 and others added 21 commits June 17, 2022 10:24
    * Usage of jboss-modules library
    * introduce ModularJarDeploymentService
    * introduce ModularClasspathService
    * introduce Gradle plugin to generate module descriptors
    * add --enable_classloader_isolation flag to start server command
    * refactor DUnit tests to optionally run with classloader isolation
    * add checks to CI to run DUnits with classloader isolation
    * updated docs to reflect new Classloader isolation
Changed logic to restrict to "root-only" jars.
StartServerCommand and StartServerCommandTest amended
Fixes resulting from new plugin build approach
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jdk8 add this label to get Linux JDK8 PR checks too windows add this label to get Windows JDK11 PR checks too windows-jdk8 add this label to get Windows JDK8 PR checks too
Projects
None yet
9 participants