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

sofa-bolt com.alipay.remoting.rpc.protocol.UserProcessor should have an extension interface to assign UserProccessor ClassLoader #222

Closed
guanchao-yang opened this issue May 18, 2020 · 0 comments
Assignees
Labels
dev:feature New feature or request
Milestone

Comments

@guanchao-yang
Copy link
Member

Describe the bug

When using sofa-bolt , user can extends com.alipay.remoting.rpc.protocol.SyncUserProcessor and implement some interface such as com.alipay.antschedulerclient.rpc.processor.ClusterJobSplitProcessor#getExecutor to assign User ThreadPool. But when self defined user processor is run in sofa-bolt code com.alipay.remoting.rpc.protocol.RpcRequestProcessor#process`:

     * @see com.alipay.remoting.AbstractRemotingProcessor#process(com.alipay.remoting.RemotingContext, com.alipay.remoting.RemotingCommand, java.util.concurrent.ExecutorService)
     */
    @Override
    public void process(RemotingContext ctx, RpcRequestCommand cmd, ExecutorService defaultExecutor)
                                                                                                    throws Exception {
        
        UserProcessor userProcessor = ctx.getUserProcessor(cmd.getRequestClass());
        // simplify codes      
        // Till now, if executor still null, then try default
        if (executor == null) {
            executor = (this.getExecutor() == null ? defaultExecutor : this.getExecutor());
        }

        // use the final executor dispatch process task
        executor.execute(new ProcessTask(ctx, cmd));
    }

In executor.execute(new ProcessTask(ctx, cmd)) codes will call com.alipay.remoting.serialization.HessianSerializer#deserialize ,but when the Thread.currentThread().getContextClassLoader() is Container or Web Framework ClassLoader may cause deserialize failed ,and cause some user classed deserialized as java.util.HashMap.

Expected behavior

Thread.currentThread().getContextClassLoader() can self-defined and when submit user processor in code executor.execute(new ProcessTask(ctx, cmd)); using Thread.currentThread().setContextClassLoader to set and com.alipay.remoting.serialization.HessianSerializer#deserialize can deserialize actual user expected classes

Actual behavior

Thread.currentThread().setContextClassLoadercan't be set andcom.alipay.remoting.serialization.HessianSerializer#deserializefailed ,user classes are deserialized asjava.util.HashMap` using Hessian

Steps to reproduce

Have Self defined ClassLoader Container and Thread.currentThread().getContextClassLoader() not expected

Environment

All SOFABolt version have the same problems and can't set user Thread Context ClassLoader

  • SOFABolt version:
  • JVM version (e.g. java -version):
  • OS version (e.g. uname -a):
  • Maven version:
  • IDE version:
@dbl-x dbl-x self-assigned this May 18, 2020
@dbl-x dbl-x added the dev:feature New feature or request label Jul 1, 2020
@dbl-x dbl-x added this to the 1.6.2 release milestone Jul 1, 2020
dbl-x added a commit to dbl-x/sofa-bolt that referenced this issue Jul 6, 2020
@dbl-x dbl-x mentioned this issue Jul 7, 2020
cytnju pushed a commit that referenced this issue Jul 13, 2020
* fix code style

* fix #210

* fix #210

* fix #222

* support check and create connection in async way

* return false if no connection and try create in async way

* switch ClassLoader in try-finally
@dbl-x dbl-x closed this as completed Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev:feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants