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

fix ConcurrentModificationException #425

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

jubatusd
Copy link
Contributor

@jubatusd jubatusd commented Mar 21, 2024

I noticed that the use of clearableRefs in the method clearAllReferences method is incorrect.
The type of clearableRefs is synchronizedList. Although the safeList is thread-safe, its iterator is not.
According to Javadocs :

It is imperative that the user manually synchronize on the returned collection when traversing it via Iterator, Spliterator or Stream

I think this is the root cause of the occurrence of ConcurrentModificationException.

To analyze more specifically, why is there a very small chance of a ConcurrentModificationException occurring

When the use of heap memory exceeds the limit, notificationListener.notifyHeapAboveThreshold() will be called. It runs the clearing of references asynchronously there, which involves modifying clearableRefs.

Is it possible that when the clearAllReferences() method is called, some of the threads initiated by the notificationListener.notifyHeapAboveThreshold() method have not finished executing. This leads to the use of clearableRefs.spliterator() to generate the iterator of clearableRefs for traversal, and clearableRefs is modified by other threads again.
image

Resolves #424

@jubatusd jubatusd closed this Mar 21, 2024
@jubatusd jubatusd changed the title fix : add synchronized on the returned list when traversing it Splite… fix ConcurrentModificationException Mar 21, 2024
@jubatusd jubatusd reopened this Mar 21, 2024
Copy link
Contributor

@mpollmeier mpollmeier left a comment

Choose a reason for hiding this comment

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

makes sense, thank you!

@mpollmeier mpollmeier merged commit 0810c40 into ShiftLeftSecurity:master Mar 21, 2024
1 check passed
@mpollmeier
Copy link
Contributor

I'll push the upgrade through cpg and joern

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.

ConcurrentModificationException
2 participants