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

[BACKPORT] Fix constructor cache in ClassLoaderUtil #13511

Conversation

viliam-durina
Copy link
Contributor

@viliam-durina viliam-durina commented Jul 31, 2018

Fixes #13509

Backport of #13512

@@ -102,13 +102,13 @@ private ClassLoaderUtil() {
return newInstance(klass, classLoader, className);
}

public static <T> T newInstance(Class<T> klass, ClassLoader classLoader, String className) throws Exception {
public static <T> T newInstance(Class<T> klass, ClassLoader unused, String className) throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be ok to remove this parameter altogether?

Copy link
Contributor

@jerrinot jerrinot left a comment

Choose a reason for hiding this comment

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

This effectively disable caching in the case of delegating classloaders.

The code should populate the constructor cache with the classloader which was used to load the class. because that's what the cache lookup code is using.

however you are now populating the cache with a classloader which was used to define the class.

@viliam-durina viliam-durina changed the title Fix constructor cache in ClassLoaderUtil [WIP] Fix constructor cache in ClassLoaderUtil Jul 31, 2018
@cangencer cangencer added the Source: Jet Issues/PRs needed for Jet, but in other modules than Jet label Aug 30, 2018
- The constructor will be cached with the class loader (CL) that
returned the class. Fixes hazelcast#13509

- The same CL could be queried multiple times (performance problem)

- the thread's context CL wasn't used at all for `com.hazelcast` package.
This problem partly remains, see comment in code.

- add documentation

- make the oddly-defined 3-arg `newInstance` method private
@cangencer cangencer force-pushed the constructor-cache-fix-backport branch from ee1cdfb to 7d18f7d Compare September 3, 2018 08:34
@cangencer cangencer changed the title [WIP] Fix constructor cache in ClassLoaderUtil [BACKPORT] Fix constructor cache in ClassLoaderUtil Sep 3, 2018
@mmedenjak
Copy link
Contributor

Closing in favour of #13679

@mmedenjak mmedenjak closed this Sep 3, 2018
@viliam-durina viliam-durina deleted the constructor-cache-fix-backport branch October 15, 2019 14:40
@mmedenjak mmedenjak added the Source: Internal PR or issue was opened by an employee label Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Source: Internal PR or issue was opened by an employee Source: Jet Issues/PRs needed for Jet, but in other modules than Jet Team: Core Type: Defect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants