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 a possible race issue #884

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

Comments

Projects
None yet
3 participants
@baigd

baigd commented Sep 22, 2015

Hi, Developers of dianping/cat,

I am writing to report a race issue on use of ConcurrentHashMap. The issue is reported by our tool in an automatic way. Although manually confirmed, it would be a false positive, 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 it is a real problem. For more information, please refer to our website: http://sav.sutd.edu.sg/?page_id=2845

File:
dianping/cat/cat-core/src/main/java/com/dianping/cat/report/DefaultReportManager.java
Location: (104/100, 61/153)
Description:
The synchronized block from line 99 to line 106 contains a get-then-put operation on "m_reports". If the intention of this locking is to guarantee the atomicity of the get-then-put operation, then the remove operation in line 61 may break this atomicity since it is not properly locked. The put operation in line 153 may have the same effect though it may not be serious. Relying on the ConcurrentHashMap to ensure exclusive access is dangerous since ConcurrentHashMap has no guarantee of exclusive access.

@qmwu2000

This comment has been minimized.

Show comment
Hide comment
@qmwu2000

qmwu2000 Sep 22, 2015

Contributor

Thanks for notification! We will fix this kind of issues.

发自我的 iPhone

在 2015年9月22日,15:21,baigd notifications@github.com 写道:

Hi, Developers of dianping/cat,

I am writing to report a race issue on use of ConcurrentHashMap. The issue is reported by our tool in an automatic way. Although manually confirmed, it would be a false positive, 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 it is a real problem. For more information, please refer to our website: http://sav.sutd.edu.sg/?page_id=2845

File:
dianping/cat/cat-core/src/main/java/com/dianping/cat/report/DefaultReportManager.java
Location: (104/100, 61/153)
Description:
The synchronized block from line 99 to line 106 contains a get-then-put operation on "m_reports". If the intention of this locking is to guarantee the atomicity of the get-then-put operation, then the remove operation in line 61 may break this atomicity since it is not properly locked. The put operation in line 153 may have the same effect though it may not be serious. Relying on the ConcurrentHashMap to ensure exclusive access is dangerous since ConcurrentHashMap has no guarantee of exclusive access.


Reply to this email directly or view it on GitHub.

Contributor

qmwu2000 commented Sep 22, 2015

Thanks for notification! We will fix this kind of issues.

发自我的 iPhone

在 2015年9月22日,15:21,baigd notifications@github.com 写道:

Hi, Developers of dianping/cat,

I am writing to report a race issue on use of ConcurrentHashMap. The issue is reported by our tool in an automatic way. Although manually confirmed, it would be a false positive, 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 it is a real problem. For more information, please refer to our website: http://sav.sutd.edu.sg/?page_id=2845

File:
dianping/cat/cat-core/src/main/java/com/dianping/cat/report/DefaultReportManager.java
Location: (104/100, 61/153)
Description:
The synchronized block from line 99 to line 106 contains a get-then-put operation on "m_reports". If the intention of this locking is to guarantee the atomicity of the get-then-put operation, then the remove operation in line 61 may break this atomicity since it is not properly locked. The put operation in line 153 may have the same effect though it may not be serious. Relying on the ConcurrentHashMap to ensure exclusive access is dangerous since ConcurrentHashMap has no guarantee of exclusive access.


Reply to this email directly or view it on GitHub.

@youyong205 youyong205 closed this Oct 8, 2015

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