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 potential exception by removing put outside computeIfAbsent #325

Closed
wants to merge 1 commit into from

Conversation

oicu0619
Copy link

this.classes is a ConcurrentHashMap. The originalCode try to put something into this.classes inside computeIfAbsent. computeIfAbsent will acquire lock of certain slot of the ConcurrentHashMap. If clazz.qualifiedName end up in the same slot, it will also try to accquire the same lock, which is already held by the same thread, will cause "java.lang.IllegalStateException: Recursive update"
demo.zip
This is a small jar file that will trigger the excetpion.

@jaskarth jaskarth self-assigned this Oct 11, 2023
@jaskarth jaskarth added Subsystem: Writing Anything concerning how expressions are written bugfix Fixes a bug labels Oct 11, 2023
Copy link
Member

@jaskarth jaskarth left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution, and sorry that it took so long for me to get back to you! The 1.10.0 dev branch has a fix for this, but this is a nice stop-gap fix. I will merge once I set up the dev branch for the 1.9.4 release.

@jaskarth
Copy link
Member

jaskarth commented Apr 4, 2024

Hey! Sorry about not getting back sooner, but plans changed and 1.10.0 was released first, and 1.9.4 was skipped. Thanks for the contribution though! It is very much appreciated.

@jaskarth jaskarth closed this Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes a bug Subsystem: Writing Anything concerning how expressions are written
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants