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

ARTEMIS-565 Dealing with ClassLoading issues on managements. Caching … #684

Merged
merged 2 commits into from
Jul 29, 2016

Conversation

clebertsuconic
Copy link
Contributor

…provider loaded

@johnament
Copy link
Contributor

What was happening prior to this change?

@clebertsuconic
Copy link
Contributor Author

If you build and run message-count example without this fix you will see a class not found exception.
The server is not loaded worth the appClassLoader (system class loader). And the Json is always loading the class loader thing.

Management is using RMI and I have no control over the class loader. I would need to either do this is wrap every messaging call with setting the Tccl.

This is the best solution I could find. But I'm open to anything else.

@johnament
Copy link
Contributor

If you don't mind, let me take a look over the weekend. This sounds like more of an issue with the geronimo spec jar, which we can get changes into.

https://github.com/apache/geronimo-specs/blob/trunk/geronimo-json_1.0_spec/src/main/java/javax/json/spi/JsonProvider.java#L67

@clebertsuconic
Copy link
Contributor Author

i thought about changing JsonProvider to cache the loaded so this wouldn't be an issue (I could load it under the right class loader once)

@clebertsuconic
Copy link
Contributor Author

Will wait for you then.

@johnament
Copy link
Contributor

Mmmph. So I can fix the spec jar for geronimo, but the problem will persist in the other spec jars, e.g. the same problem happens w/ the RI JAR and the JBoss JAR.

Minor issue, I would do this as the loader method, to avoid casting

   private static JsonProvider loadProvider() {
      return AccessController.doPrivileged(new PrivilegedAction<JsonProvider>() {
         @Override
         public JsonProvider run() {
            ClassLoader originalLoader = Thread.currentThread().getContextClassLoader();
            try {
               Thread.currentThread().setContextClassLoader(JsonProvider.class.getClassLoader());
               return JsonProvider.provider();
            } finally {
               Thread.currentThread().setContextClassLoader(originalLoader);
            }
         }
      });

   }

@clebertsuconic
Copy link
Contributor Author

@johnament I wouldn't need this whole thing if this was merged. Just sent the PR:

apache/geronimo-specs#4

I"m not sure about the reasoning for loading the class every time. It seems silly to me... so I guess this PR should be an improvement. You know that project better than I do.

@johnament
Copy link
Contributor

At this point, this is the best solution to ensure that it works across different impls and spec jars. +1

@asfgit asfgit merged commit ea1651b into apache:master Jul 29, 2016
asfgit pushed a commit that referenced this pull request Jul 29, 2016
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.

3 participants