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

Report possible race issues #251

Closed
baigd opened this Issue Sep 22, 2015 · 1 comment

Comments

2 participants
@baigd

baigd commented Sep 22, 2015

Hi, Developers of mrniko/redisson,

I am writing to report two race issues on use of ConcurrentHashMap. The issues are reported by our tool in an automatic way. Although manually confirmed, they would be false positives, given we do not know the specification of the program. We would very appreciate if you could check below for details and confirm with us whether they are real problems. For more information, please refer to our website: http://sav.sutd.edu.sg/?page_id=2845

File:
mrniko/redisson/src/main/java/org/redisson/RedissonCountDownLatch.java
Location: Line (66, 120)
Description:
Line 120 is in the synchronized block on "ENTRIES". If the intention is to guarantee exclusive access for the !containsKey checking on "ENTRIES" and to ensure the atomicity of the checking and unsubscription (line 120,121), then the write operations on "ENTRIES" in line 66, 115, 117, 136 may break this. Relying on the ConcurrentHashMap to ensure exclusive access is dangerous since ConcurrentHashMap has no guarantee of exclusive access.

File:
mrniko/redisson/src/main/java/org/redisson/RedissonLock.java
Location: Line (75, 113)
Description:
Same as above.

@mrniko

This comment has been minimized.

Show comment
Hide comment
@mrniko

mrniko Sep 22, 2015

Member

Hi! I have a look at those lines but didn't find nothing extraordinary there. Sync sections are used to avoid parallel execution of subscribe and unsubscribe commands. RedissonLock and RedissonCountDownLatch have concurrent tests also and they always pass it.

As for lock-free implementation of entry's resources counter managed by release/aquire methods it's done a little bit ugly with while statements and so on... Anyway I had refactored this code. Thank you for attracting my attention to this code.

I found your project is very impressive. Because concurrent issues scanning is very important thing for all kind of projects not only open-source.

Member

mrniko commented Sep 22, 2015

Hi! I have a look at those lines but didn't find nothing extraordinary there. Sync sections are used to avoid parallel execution of subscribe and unsubscribe commands. RedissonLock and RedissonCountDownLatch have concurrent tests also and they always pass it.

As for lock-free implementation of entry's resources counter managed by release/aquire methods it's done a little bit ugly with while statements and so on... Anyway I had refactored this code. Thank you for attracting my attention to this code.

I found your project is very impressive. Because concurrent issues scanning is very important thing for all kind of projects not only open-source.

@mrniko mrniko closed this Sep 28, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment