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

Use NarClassLoader in protocol hander callbacks. #11270

Closed
Jason918 opened this issue Jul 9, 2021 · 2 comments · Fixed by #11276
Closed

Use NarClassLoader in protocol hander callbacks. #11270

Jason918 opened this issue Jul 9, 2021 · 2 comments · Fixed by #11276
Labels
type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages

Comments

@Jason918
Copy link
Contributor

Jason918 commented Jul 9, 2021

Is your enhancement request related to a problem? Please describe.
I am working on a new protocol handler for an existing mq implementation.
ClassNotFoundException occurs when I am calling methods like com.fastjson.JSON.parseObject("some json str", MyConfig.class). This happens because the MyConfig.class is dynamically loaded by a NarClassLoader in protocol hander mechanism, but current thread calling ProtocolHandler::initialize uses the default class load in pulsar.

Describe the solution you'd like
IMHO, Maybe it's not very elegant for protocol handler to handle class loader issue in its own project?

As there is an ProtocolHandlerWithClassLoader object in each protocol handlers, I am suggesting change the thread default class loader to the NarClassLoader before we call interfaces in ProtocolHandler.

Describe alternatives you've considered
Currently, I am getting the NarClassLoader through MyProtocolHandler.class.getClassLoader().
I think it's a bit of tricky.

Additional context
Add any other context or screenshots about the feature request here.

@Jason918 Jason918 added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Jul 9, 2021
@kaori-seasons
Copy link

kaori-seasons commented Jul 9, 2021

@Jason918 Do you expect to get the class loader in this way

            if (StringUtils.isNotEmpty(interfaceName)) {
                ClassLoader classLoader = null == Thread.currentThread().getContextClassLoader() ? this.getClass().getClassLoader() : Thread.currentThread().getContextClassLoader();
                Class interfaceClass = Class.forName(interfaceName, true, classLoader);
                FullServiceDefinition fullServiceDefinition = ServiceDefinitionBuilder.buildFullDefinition(interfaceClass, providerUrl.getParameters());
}

see This is how the dubbo2.7 metadata module obtains the class loader
apache/dubbo#4767

@Jason918
Copy link
Contributor Author

@complone Yes, I am currently using MyProtocolHandler.class.getClassLoader() in my protocol handler project. I am assuming it's better that we set the NarClassLoader as current thread's context class loader. You can see my PR above.

BewareMyPower pushed a commit that referenced this issue Jan 4, 2022
)

### Motivation

It's ``AddtionalServlet`` side change, like #11270 

### Modifications

Change context class loader through Thread.currentThread().setContextClassLoader(classLoader) before every protocol handler callback, and change it back to original class loader afterwards.
wuzhanpeng pushed a commit to wuzhanpeng/pulsar that referenced this issue Jan 5, 2022
…che#13501)

### Motivation

It's ``AddtionalServlet`` side change, like apache#11270 

### Modifications

Change context class loader through Thread.currentThread().setContextClassLoader(classLoader) before every protocol handler callback, and change it back to original class loader afterwards.
codelipenghui pushed a commit that referenced this issue Jan 6, 2022
)

### Motivation

It's ``AddtionalServlet`` side change, like #11270

### Modifications

Change context class loader through Thread.currentThread().setContextClassLoader(classLoader) before every protocol handler callback, and change it back to original class loader afterwards.

(cherry picked from commit 40ac793)
codelipenghui pushed a commit that referenced this issue Jan 10, 2022
)

### Motivation

It's ``AddtionalServlet`` side change, like #11270

### Modifications

Change context class loader through Thread.currentThread().setContextClassLoader(classLoader) before every protocol handler callback, and change it back to original class loader afterwards.

(cherry picked from commit 40ac793)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants