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

使用@SPI的wrapper方式报空指针异常bug #7176

Closed
zhihuayu opened this issue Feb 7, 2021 · 12 comments
Closed

使用@SPI的wrapper方式报空指针异常bug #7176

zhihuayu opened this issue Feb 7, 2021 · 12 comments
Labels
help wanted Everything needs help from contributors
Milestone

Comments

@zhihuayu
Copy link

zhihuayu commented Feb 7, 2021

Environment

  • Dubbo version: 2.7.8
  • Operating System version: windows10
  • Java version: 1.8

比如,有如下的wrapper方式的Filter,在服务启动的时候会报空指针异常。

public class DubboWrapperFilter implements Filter {

	private final Filter filter;

	public DubboWrapperFilter(Filter filter) {
		this.filter = filter;
	}

	@Override
	public Result invoke(Invoker<?> invoker, Invocation invocation) throws RpcException {
		String name = filter.getClass().getName();
		System.out.println("wrapper:" + name);
		return filter.invoke(invoker, invocation);
	}

}

经查是因为active的排序导致,如果改成如下方式,则不会报错,但是其排序可能会无效,所以希望官方能做一个完整的方案,谢谢!

@Activate
public class WrapperHelloSPI implements HelloSPI {

	private final HelloSPI spi;

	public WrapperHelloSPI(HelloSPI spi) {
		this.spi = spi;
	}

	@Override
	public void say(String say) {
		System.out.println("wapper:" + say);
		spi.say(say);
	}

}

@AlbumenJ AlbumenJ added the help wanted Everything needs help from contributors label Feb 8, 2021
@AlbumenJ
Copy link
Member

AlbumenJ commented Feb 8, 2021

@zhihuayu hi, would you pls provide more detailed information for NPE.

The situation you use wrapper and the stack trace for NPE are both needed.

@zhihuayu
Copy link
Author

@zhihuayu hi, would you pls provide more detailed information for NPE.

The situation you use wrapper and the stack trace for NPE are both needed.

非常感谢您的回复,也非常抱歉没有提供详细的堆栈信息。经调试,主要是因为duboo在初始化构建调用链时,即调用ProtocolFilterWrapper.buildInvokerChain方法时:

 private static <T> Invoker<T> buildInvokerChain(final Invoker<T> invoker, String key, String group) {
        Invoker<T> last = invoker;
        // 抛出异常
        List<Filter> filters = ExtensionLoader.getExtensionLoader(Filter.class).getActivateExtension(invoker.getUrl(), key, group);
   ......
}

其原因在ExtensionLoader的getActivateExtension方法中:

   public List<T> getActivateExtension(URL url, String[] values, String group) {
        List<T> activateExtensions = new ArrayList<>();
        List<String> names = values == null ? new ArrayList<>(0) : asList(values);
        if (!names.contains(REMOVE_VALUE_PREFIX + DEFAULT_KEY)) {
            getExtensionClasses();
            for (Map.Entry<String, Object> entry : cachedActivates.entrySet()) {
                String name = entry.getKey();
                Object activate = entry.getValue();

                String[] activateGroup, activateValue;

                if (activate instanceof Activate) {
                    activateGroup = ((Activate) activate).group();
                    activateValue = ((Activate) activate).value();
                } else if (activate instanceof com.alibaba.dubbo.common.extension.Activate) {
                    activateGroup = ((com.alibaba.dubbo.common.extension.Activate) activate).group();
                    activateValue = ((com.alibaba.dubbo.common.extension.Activate) activate).value();
                } else {
                    continue;
                }
                if (isMatchGroup(group, activateGroup)
                        && !names.contains(name)
                        && !names.contains(REMOVE_VALUE_PREFIX + name)
                        && isActive(activateValue, url)) {
                    activateExtensions.add(getExtension(name));
                }
            }
            // 排序抛出异常
            activateExtensions.sort(ActivateComparator.COMPARATOR);
        }
       ......
    }

实际上,抛出的空指针异常是在ActivateComparator.COMPARATOR比较排序的时候抛出的,如下所示ActivateComparator.parseActivate方法中,由于没有考虑Activate是否存在的情况,所以会抛出控制在异常:

  private ActivateInfo parseActivate(Class<?> clazz) {
        ActivateInfo info = new ActivateInfo();
        if (clazz.isAnnotationPresent(Activate.class)) {
            Activate activate = clazz.getAnnotation(Activate.class);
            info.before = activate.before();
            info.after = activate.after();
            info.order = activate.order();
        } else {
            com.alibaba.dubbo.common.extension.Activate activate = clazz.getAnnotation(
                    com.alibaba.dubbo.common.extension.Activate.class);
            //  没有考虑Activate不存在的情况,排除空指针异常
            info.before = activate.before();
            info.after = activate.after();
            info.order = activate.order();
        }
        return info;
    }

@xiaoheng1
Copy link
Contributor

应该不是这个问题吧?getActivateExtension 方法遍历的是 cachedActivates.entrySet(),而 cachedActivates 缓存的是被 Activate 注解修饰的类,所以在排序的时候,应该全部是包含 Activate 注解的类.

public List<T> getActivateExtension(URL url, String[] values, String group) {
       ...
            for (Map.Entry<String, Object> entry : cachedActivates.entrySet()) {
                String name = entry.getKey();
                Object activate = entry.getValue();

                String[] activateGroup, activateValue;

                if (activate instanceof Activate) {
                    activateGroup = ((Activate) activate).group();
                    activateValue = ((Activate) activate).value();
                } else if (activate instanceof com.alibaba.dubbo.common.extension.Activate) {
                    activateGroup = ((com.alibaba.dubbo.common.extension.Activate) activate).group();
                    activateValue = ((com.alibaba.dubbo.common.extension.Activate) activate).value();
                } else {
                    continue;
                }
                if (isMatchGroup(group, activateGroup)
                        && !names.contains(name)
                        && !names.contains(REMOVE_VALUE_PREFIX + name)
                        && isActive(activateValue, url)) {
                    activateExtensions.add(getExtension(name));
                }
            }
            activateExtensions.sort(ActivateComparator.COMPARATOR);
        }

}

@zhihuayu
Copy link
Author

ctivates.entrySet()) {

对的,但是Activate 注解修饰的类可以被没有Activate注解的wrapper类所修饰,原理是在调用getExtension(name)方法时会进行包装,包装之后再排序,所以会有空指针异常。

@AlbumenJ
Copy link
Member

@zhihuayu thanks for your explanation

I have tried to reroduce this situation, and I found that the main reason which causes NPE in activateExtensions.sort(ActivateComparator.COMPARATOR); is this real Activate extension has been wrapped by getExtension(name) which will make extension being wrapped and the original Activate annotation unable to being fetch by ActivateComparator.COMPARATOR.

image

In order to slove this problem, maybe we can make getExtension(name) being called after sorted? Or would have any ideas for this?

We can fix it in the next release.

@AlbumenJ AlbumenJ added this to the 2.7.10 milestone Feb 18, 2021
@zhihuayu
Copy link
Author

@AlbumenJ 非常感谢您的回复
是的,排序之后再调用getExtension(name) 方法可以解决这个问题。我觉得,可以利用TreeMap数据结构的排序特性来完成这个功能,这样就只需要简单的修改ExtensionLoader.getActivateExtension方法以及排序策略类ActivateComparator即可,ExtensionLoader.getActivateExtension方法修改如下所示:

image

而ActivateComparator实现的Comparator的泛型为Object改成Class类型即可,如下所示:

修改2

上述修改方式仅是个人建议,仅供参考,再次感谢您的回复!

xiaoheng1 pushed a commit to xiaoheng1/incubator-dubbo that referenced this issue Feb 22, 2021
@chickenlj
Copy link
Contributor

@zhihuayu @xiaoheng1 非常欢迎提个 pr 到主干仓库

@xiaoheng1
Copy link
Contributor

already in process.

@zhihuayu
Copy link
Author

@zhihuayu @xiaoheng1 非常欢迎提个 pr 到主干仓库

@xiaoheng1 已经提交了,谢谢!

@chickenlj
Copy link
Contributor

不好意思,我没在这个 issue 中看到相关的 pull request 链接那

zhihuayu added a commit to zhihuayu/dubbo that referenced this issue Feb 22, 2021
@zhihuayu
Copy link
Author

@chickenlj 刚才已经提交pull request。

@AlbumenJ
Copy link
Member

Related pr #7234

xiaoheng1 pushed a commit to xiaoheng1/incubator-dubbo that referenced this issue Mar 4, 2021
xiaoheng1 added a commit to xiaoheng1/incubator-dubbo that referenced this issue Mar 4, 2021
@AlbumenJ AlbumenJ modified the milestones: 2.7.10, 2.7.11 Apr 2, 2021
vio-lin pushed a commit to vio-lin/incubator-dubbo that referenced this issue Feb 22, 2023
…pache#7326)

* fix Use @SPI's wrapper method to report a null pointer exception bug. see apache#7176.

(cherry picked from commit df6d6ad)

* fix apache#7176 Use @SPI's wrapper method to report a null pointer exception bug

* recover wildcard import

* delete issue link

* add new spi interface.

* fix testActivateComparator test case.

Co-authored-by: xiaoheng <xiaoyu>

(cherry picked from commit a10d7f3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Everything needs help from contributors
Projects
None yet
Development

No branches or pull requests

4 participants