Zookeeper ACL机制中ProviderRegistry的设计缺陷

本文主要讨论 Zookeeper 的 ACL 机制实现中使用到的 ProviderRegistry 类的设计不合理之处。

因此，先简单介绍下 ACL 机制，及相关的类

ACL

ZooKeeper 的 ACL 可针对 znodes 设置相应的认证方式和权限信息。ACL 数据的表示格式为：schema:id:permissions

schema 的是通过 AuthenticationProvider 实现的，每种 AuthenticationProvider 对应一个 schema，对应关系如下

AuthenticationProvider & schema
DigestAuthenticationProvider	digest
IPAuthenticationProvider	ip
SASLAuthenticationProvider	sasl
X509AuthenticationProvider	x509

此处不详细讨论 ACL

AuthenticationProvider

每一种认证方式均需要实现 AuthenticationProvider 接口来支持一种新的 schema。

Zookeeper 对于AuthenticationProvider 的设计描述是 Pluggable ZooKeeper authentication，即可插拔的。

ZooKeeper runs in a variety of different environments with various different authentication schemes, so it has a completely pluggable authentication framework. Even the builtin authentication schemes use the pluggable authentication framework.

百度翻译一下：ZooKeeper使用各种不同的身份验证方案在各种不同的环境中运行，因此它有一个完全可插入的身份验证框架。即使是内置的身份验证方案也使用可插入的身份验证框架。

ProviderRegistry

顾名思义，这个类，是 AuthenticationProvider 的注册中心，负责维护 AuthenticationProvider。

看下这个类的设计

• 这个类的所有方法和字段都是静态的 static
• initialize 方法会
  • 先创建 IPAuthenticationProvider 和 DigestAuthenticationProvider
  • 然后再扫描 System property 中以 zookeeper.authProvider. 开头的配置，再通过反射创建用户自定义的 AuthenticationProvider
  • 所有的 AuthenticationProvider 都会存入 authenticationProviders 中
• getProvider 用于通过 scheme 获取 AuthenticationProvider

这段代码如下

    public static void initialize() {
        synchronized (ProviderRegistry.class) {
            IPAuthenticationProvider ipp = new IPAuthenticationProvider();
            authenticationProviders.put(ipp.getScheme(), ipp);

            if (DigestAuthenticationProvider.isEnabled()) {
                DigestAuthenticationProvider digp = new DigestAuthenticationProvider();
                authenticationProviders.put(digp.getScheme(), digp);
            }

            Enumeration<Object> en = System.getProperties().keys();
            while (en.hasMoreElements()) {
                String k = (String) en.nextElement();
                addOrUpdateProvider(k);
            }
            initialized = true;
        } 
    }
    
    public static AuthenticationProvider getProvider(String scheme) {
        if (!initialized) {
            initialize();
        }
        return authenticationProviders.get(scheme);
    }

发现问题

不知道看了这段代码，你有没有发现什么问题，反正我发现了

getProvider 中会先使用标识符 initialized 判断是否初始化过，如果没有，就进行初始化，但是 initialize 方法中却没有判断否初始化，而是直接进行了初始化。

也就是说在并发时，initialize 方法可能会被调用到多次。那什么情况下会并发的调用到 initialize 呢？

看了一下这个方法的引用，发现，都是在 ZookeeperServer 启动时，会调用这个初始化方法，也就是说，不会有并发调用的情况。

可是如果咱们不考虑外部是如何调用的（调用的时序，是否并发等），这个类的设计确实存在问题，应该像下面这样，在执行真正的初始化前做一次 check

提出问题

想给官方提个 PR，怎么能够暴露出这个设计带来的问题呢？

想到了 ZooKeeperServerEmbedded，嵌入式的 zkServer，支持一个 JVM 中启动多个实例。选择其中一个看些调用时序

想象一下，如果我们在一个 JVM 中并发的启动多个 ZooKeeperServerEmbedded 的实例，那就有可能导致 ProviderRegistry 重复的初始化，现在可以提 PR 了。

我提了个 PR，也被合并了，详细如下

ZOOKEEPER-4549: ProviderRegistry may be repeatedly initialized #1888

更多问题

本来以为这个就结束了，谁知道。。。

测试用例没跑通， CI #1043。原因可以参照 eolivelli 大佬的回答，大概意思就是测试用例中运行了多个 zkServer，但是 ProviderRegistry 中静态的变量，使得这类用例发生了混乱。

于是我这个 PR 的提交被回滚了。。。

我也发表了我的看法，就是，每个 zkServer 都应当维护一个自己独有的 ProviderRegistry 对象，而不是像现在这样共享。

但是这个改动比较大，等待大佬们反馈，看如何修改吧。