Skip to content
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

Fixes ConcurrentModificationException mentioned in Issue #47 #50

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gwilli
Copy link

@gwilli gwilli commented Nov 28, 2017

This crash was our app's most common crash prior to making this change.

Fatal Exception: java.util.ConcurrentModificationException
       at java.util.HashMap$HashIterator.nextEntry(HashMap.java:851)
       at java.util.HashMap$KeyIterator.next(HashMap.java:885)
       at com.franmontiel.persistentcookiejar.cache.SetCookieCache$SetCookieCacheIterator.next(SetCookieCache.java:67)
       at com.franmontiel.persistentcookiejar.cache.SetCookieCache$SetCookieCacheIterator.next(SetCookieCache.java:52)

After this change, we've had zero ConcurrentModificationExceptions and our crash-free sessions are up to over 99%.

Hashset is not thread-safe and the use of the synchronized keyword was still allowing the crash to happen in the iterator's next() method.

We are using this fork in our production app with no issues.

@tmm1
Copy link

tmm1 commented May 7, 2018

@franmontiel ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants