CodisLabs / jodis

A java client for codis based on Jedis and Curator
MIT License
217 stars 97 forks source link

jodis原子性问题 #46

Closed lvdelu closed 7 years ago

lvdelu commented 7 years ago

for (;;) { int current = nextIdx.get(); int next = current >= pools.size() - 1 ? 0 : current + 1; if (nextIdx.compareAndSet(current, next)) { return pools.get(next).pool.getResource(); } }

这段源码存在原子性问题

Apache9 commented 7 years ago

啥问题?

lvdelu commented 7 years ago

@Override public Jedis getResource() { ImmutableList pools = this.pools; if (pools.isEmpty()) { throw new JedisException("Proxy list empty"); }

        for (;;) {
            int current = nextIdx.get();
            int next = current >= pools.size() - 1 ? 0 : current + 1;
            if (nextIdx.compareAndSet(current, next)) {
                return pools.get(next).pool.getResource();
            }
        }

}

这段代码存在原子性问题

lvdelu commented 7 years ago

应该改成下面这样 @Override public Jedis getResource() { ImmutableList pools = this.pools; if (pools.isEmpty()) { throw new JedisException("Proxy list empty"); } synchronized (this.pools) { for (;;) { int current = nextIdx.get(); int next = current >= pools.size() - 1 ? 0 : current + 1; if (nextIdx.compareAndSet(current, next)) { return pools.get(next).pool.getResource(); } } } }

Apache9 commented 7 years ago

你是实际遇到问题了?还是看代码看到这里觉得有问题? 那个pools是个ImmutableList,生成了就不会变了,每次更新列表的时候都是创建一个新的ImmutableList,不会改之前的。

lvdelu commented 7 years ago

看代码啊,pools是原子的,for 中的死循环不是原子操作啊

Apache9 commented 7 years ago

那个nextIdx是AtomicInteger啊,标准的CAS原子操作

lvdelu commented 7 years ago

current不是原子的啊

Apache9 commented 7 years ago

current是个局部变量啊,只有本线程能访问到。。。

lvdelu commented 7 years ago

我错了,囧