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.

Sign up for GitHub

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

Jump to bottom

Fix ConcurrentModificationException #3561

Merged
merged 5 commits into from
Aug 5, 2021
Merged

Fix ConcurrentModificationException #3561

merged 5 commits into from
Aug 5, 2021

Conversation

jeremylong
Copy link
Owner

Initial fix for ConcurrentModificationException introduced by #3466.

@boring-cyborg boring-cyborg bot added the core changes to core label Aug 4, 2021
@jeremylong jeremylong requested a review from aikebah August 4, 2021 12:34
@aikebah
Copy link
Collaborator

aikebah commented Aug 4, 2021

Same fix is needed for DependencyMergingAnalyzer at lines 147-149.

@aikebah
Copy link
Collaborator

aikebah commented Aug 4, 2021

Sorry... overlooked that file in the changelog... It's already in there. So that leaves only FalsePostiveAnalyzer.removeBadMatches which removes items from vulnerableSoftwareIdentifiers while while iterating over them

exception: java.util.ConcurrentModificationException

java.util.TreeMap$PrivateEntryIterator.nextEntry(TreeMap.java:1211)
java.util.TreeMap$KeyIterator.next(TreeMap.java:1265)
java.util.Collections$UnmodifiableCollection$1.next(Collections.java:1044)
org.owasp.dependencycheck.analyzer.FalsePositiveAnalyzer.removeBadMatches(FalsePositiveAnalyzer.java:281)

aikebah
aikebah requested changes Aug 4, 2021
View reviewed changes
Copy link
Collaborator

@aikebah aikebah left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look good to me, and also get rid of a potentially dangerous state-modifying peek that was signalled by SonarLint.

When a FalsePositiveAnalyzer#removeBadMatches fix gets added as far as I can see no further ConcurrentModifcationExceptions should happen (all other uses of the Set-getters appear to be read-only usage, already employ a defensive copy or imlement a 'gather' phase followed by an 'update' phase in the code)

@jeremylong jeremylong requested a review from aikebah August 5, 2021 11:09
aikebah
aikebah previously approved these changes Aug 5, 2021
View reviewed changes
Copy link
Collaborator

@aikebah aikebah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jeremylong jeremylong merged commit ac81193 into main Aug 5, 2021
@jeremylong jeremylong added this to the 6.2.3 milestone Aug 5, 2021
@jeremylong jeremylong deleted the treeSetFix branch August 5, 2021 12:01
@aikebah aikebah mentioned this pull request Aug 31, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aikebah aikebah aikebah left review comments

Assignees
No one assigned
Labels
core changes to core
Projects
None yet
Milestone
6.3.0
Development

Successfully merging this pull request may close these issues.
2 participants
@jeremylong @aikebah