Skip to content

It throws java.util.ConcurrentModificationException when i use RedisLockRegistry #2983

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

Closed
Farwell-Liu opened this issue Jul 4, 2019 · 5 comments
Labels
status: duplicate There is already an issue similar to this. The link to it should be present status: waiting-for-reporter Needs a feedback from the reporter

Comments

@Farwell-Liu
Copy link

I am using RedisLockRegistry for distributed lock. but sometimes it throws java.util.ConcurrentModificationException when I unlock. I have wrote a demo code as below:
In this demo I did not use the exlusion of distributed lock.

public class MyThread extends Thread {

    private RedisConnectionFactory redisConnectionFactory;
    private int number;

    public MyThread(RedisConnectionFactory redisConnectionFactory, int number) {
        this.redisConnectionFactory = redisConnectionFactory;
        this.number = number;
    }

    @Override
    public void run() {

        System.out.println("start! number" + number + " thread info is:" + Thread.currentThread().getId() + "," + Thread.currentThread().getName());
        RedisLockRegistry lockRegistryA = new RedisLockRegistry(redisConnectionFactory, "MyRegKeyA", 200 * 1000);
        Lock lockA = null;
        for (int i = 0; i < 1000; i++) {
            lockA = lockRegistryA.obtain(String.valueOf(number));
            lockA.lock();

            lockA.unlock();
        }
        lockRegistryA.expireUnusedOlderThan(-1000);
        System.out.println("end! number" + number + " thread info is:" + Thread.currentThread().getId() + "," + Thread.currentThread().getName());

    }
}

and this is caller code:

for (int i = 0; i < 10000; i++) {
            if (i % 1000 == 0)
                System.out.println(Thread.currentThread().getName() + " " + i);
            if (i == 30) {
                Thread myThread1 = new MyThread(redisConnectionFactory, 1);
                Thread myThread2 = new MyThread(redisConnectionFactory, 2);
                myThread1.start();
                myThread2.start();

                try {
                    Thread.sleep(100);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }
        }

here are the logs:

http-nio-9090-exec-5 0
start! number1 thread info is:75,Thread-7
Exception in thread "Thread-8" java.util.ConcurrentModificationException
start! number2 thread info is:76,Thread-8
	at java.base/java.util.HashMap.computeIfAbsent(HashMap.java:1134)
	at org.springframework.integration.redis.util.RedisUtils.isUnlinkAvailable(RedisUtils.java:61)
	at org.springframework.integration.redis.util.RedisLockRegistry$RedisLock.removeLockKey(RedisLockRegistry.java:332)
	at org.springframework.integration.redis.util.RedisLockRegistry$RedisLock.unlock(RedisLockRegistry.java:316)
	at com.example.farwellspringboot.Util.MyThread.run(MyThread.java:31)
http-nio-9090-exec-5 1000
http-nio-9090-exec-5 2000
http-nio-9090-exec-5 3000
http-nio-9090-exec-5 4000
http-nio-9090-exec-5 5000
http-nio-9090-exec-5 6000
http-nio-9090-exec-5 7000
http-nio-9090-exec-5 8000
http-nio-9090-exec-5 9000
end! number1 thread info is:75,Thread-7

Is it a bug? Could you help me please? thank you very much!

@garyrussell
Copy link
Contributor

garyrussell commented Jul 4, 2019

What version are you using?

This is fixed in 5.1.5.

@garyrussell garyrussell added status: waiting-for-reporter Needs a feedback from the reporter status: duplicate There is already an issue similar to this. The link to it should be present labels Jul 4, 2019
@Farwell-Liu
Copy link
Author

I am using 5.1.3.RELEASE, OK, thank you very much!

@Farwell-Liu
Copy link
Author

After I update the package from 5.1.3.RELEASE to 5.1.6.RELEASE, It seems that the exception never occured in my demo above. however, It still throws ConcurrentModificationException rarely in my project. And it seems that the probability of occurrence is lower then before.

java.util.ConcurrentModificationException: 
	at java.base/java.util.HashMap.computeIfAbsent(HashMap.java:1134)
	at org.springframework.integration.redis.util.RedisUtils.isUnlinkAvailable(RedisUtils.java:60)
	at org.springframework.integration.redis.util.RedisLockRegistry$RedisLock.removeLockKey(RedisLockRegistry.java:332)
	at org.springframework.integration.redis.util.RedisLockRegistry$RedisLock.unlock(RedisLockRegistry.java:316)
	at com.vcredit.platorcommon.model.SyncDataHelper.executeSyncData(SyncDataHelper.java:262)

the code in SyncDataHelper.java:262 is:
lock.unlock()

@Farwell-Liu Farwell-Liu reopened this Jul 8, 2019
@garyrussell
Copy link
Contributor

I don't see how that's possible; it is now a Collections.synchronizedMap and putIfAbsent is synchronized

        @Override
        public V computeIfAbsent(K key,
                Function<? super K, ? extends V> mappingFunction) {
            synchronized (mutex) {return m.computeIfAbsent(key, mappingFunction);}
        }

And it seems that the probability of occurrence is lower than before.

The "probability" is zero.

java.util.ConcurrentModificationException:
at java.base/java.util.HashMap.computeIfAbsent(HashMap.java:1134)

Your stack trace does not include the Collections$SynchronizedMap so that proves you are still using the old version.

@Farwell-Liu
Copy link
Author

Farwell-Liu commented Jul 9, 2019

I checked the stack trace and code in 5.1.3 and 5.1.6. I think you are right. Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate There is already an issue similar to this. The link to it should be present status: waiting-for-reporter Needs a feedback from the reporter
Projects
None yet
Development

No branches or pull requests

2 participants