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 #809

Closed
baigd opened this Issue Sep 22, 2015 · 2 comments

Comments

Projects
None yet
2 participants
@baigd

baigd commented Sep 22, 2015

Hi, Developers of k9mail/k-9,

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: k9mail/k-9/k9mail/src/main/java/com/fsck/k9/controller/MessagingController.java
Location: Line (4600/4603, 5131)
Description:
The get-then-put operations (line 4600, 4603) in method "getNotificationData" are synchronized on lock "notificationData". If the intention is to guarantee the atomicity of the get-then-put operations, then the remove operation in line 5130 may break this atomicity. Relying on the ConcurrentHashMap to ensure exclusive access is dangerous since ConcurrentHashMap has no guarantee of exclusive access.

@cketti

This comment has been minimized.

Show comment
Hide comment
@cketti

cketti Sep 22, 2015

Member

Thanks for doing the research and reporting this! The issue found by your tool is indeed a bug.

On a small side note: It would have been helpful if you had included the Git SHA of the revision you were analyzing.

The surrounding code also contains a few concurrency issues. But almost all of it was rewritten in PR #795. If you have some time it'd be awesome if you ran the analysis again after the feature has been merged.

Member

cketti commented Sep 22, 2015

Thanks for doing the research and reporting this! The issue found by your tool is indeed a bug.

On a small side note: It would have been helpful if you had included the Git SHA of the revision you were analyzing.

The surrounding code also contains a few concurrency issues. But almost all of it was rewritten in PR #795. If you have some time it'd be awesome if you ran the analysis again after the feature has been merged.

@cketti cketti added the bug label Sep 22, 2015

@cketti cketti self-assigned this Sep 22, 2015

@cketti

This comment has been minimized.

Show comment
Hide comment
@cketti

cketti Oct 3, 2015

Member

The new notification code was merged.

Member

cketti commented Oct 3, 2015

The new notification code was merged.

@cketti cketti closed this Oct 3, 2015

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