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

HHH-18901 #9636

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

HHH-18901 #9636

wants to merge 1 commit into from

Conversation

Sanne
Copy link
Member

@Sanne Sanne commented Jan 17, 2025

Draft solution for https://hibernate.atlassian.net/browse/HHH-18901

We don't have a good test for this, and I didn't yet link the commits to appropriate Jiras... but I'd consider it now stable enough for others to have a look.

Specifically targeting branch 6.6 for now as that's where we can reproduce it: I'll "forward port" this if you all like it.

Thanks!

@hibernate-github-bot
Copy link

hibernate-github-bot bot commented Jan 17, 2025

Thanks for your pull request!

This pull request does not follow the contribution rules. Could you have a look?

❌ The pull request title should contain at least 2 words to describe the change properly

› This message was automatically generated.

@Sanne Sanne requested a review from scottmarlow January 17, 2025 14:30
@Sanne Sanne force-pushed the HHH-18901-66-cleaned branch from d4e96fc to 43ef237 Compare January 17, 2025 14:40
Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

Well this must have been quite an adventure :) Thanks a lot for taking care of it!

I added a few comments, but this looks reasonable to merge as is.

Comment on lines +102 to +103
resolutions.remove( className );
poolCache.remove( className );
Copy link
Member

Choose a reason for hiding this comment

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

Maybe do this in deregisterClassNameAndBytes as well? Just in case...

Copy link
Member Author

Choose a reason for hiding this comment

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

just to highlight, I've ignored this comment as I'm not very comfortable with the "just in case" ..it's indeed tempting, but it's beind de-registered in a finally block already so I wonder if it's making things a bit confusing for "future us".
Feel free to add it later if you feel strongly about it.

…rrent enhancement

In particular, made it defensive against concurrent enhancement of the same resource,
or of resources that might trigger loading symbols which another thread is re-processing.
@Sanne Sanne force-pushed the HHH-18901-66-cleaned branch from 43ef237 to 7158074 Compare January 17, 2025 17:40
@Sanne Sanne marked this pull request as ready for review January 17, 2025 17:41
@Sanne
Copy link
Member Author

Sanne commented Jan 17, 2025

Revised and ready

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.

4 participants