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

Lazy initialization of static fields should be synchronized #1198

Closed
wants to merge 1 commit into from
Closed

Lazy initialization of static fields should be synchronized #1198

wants to merge 1 commit into from

Conversation

kirill-vlasov
Copy link
Contributor

This pull request is focused on resolving occurrences of Sonar rule squid:S2444 - Lazy initialization of static fields should be synchronized
You can find more information about the issue here:
https://dev.eclipse.org/sonar/coding_rules#q=squid:S2444
Please let me know if you have any questions.
Kirill Vlasov

@dreab8
Copy link
Contributor

dreab8 commented Jan 29, 2016

Hi @kirill-vlasov , can you open a Jira for this issue.

Thanks a lot

@richardfontana
Copy link

@vladmihalcea this can be merged without needing the CLA. Thanks!

@vladmihalcea
Copy link
Contributor

@kirill-vlasov The SessionFactory bootstrap is meant to be executed from within a single thread. After it starts, the SessionFactory can be used by multiple Threads, so I'm not sure what this change will buy us. Adding synchronized when unnecessary can incur some small overhead that might not bring us any benefit. For this reason, I think we should close this PR.

@@ -150,7 +150,7 @@ else if ( member instanceof Field ) {

private static Method memberMethod;

private static Member toMember(XProperty xProperty) {
private static synchronized Member toMember(XProperty xProperty) {
Copy link
Member

Choose a reason for hiding this comment

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

Good catch, something is fishy here! Yet synchronizing is not the best solution, as it would introduce a contention point while it's not necessary.
The memberMethod should be made final and initialized statically.

Also, would be nice to have fields at the beginning of the class.

@Sanne
Copy link
Member

Sanne commented Apr 28, 2016

@richardfontana thanks! It's great to see you around, even more helping so directly.

@vladmihalcea
Copy link
Contributor

I created HHH-10715 for analyzing static field usage.

@Sanne
Copy link
Member

Sanne commented Oct 25, 2020

I was just checking for similar patterns, and I'm glad to say that most issues had already been resolved. I just fixed the remaining cases.

@Sanne Sanne closed this Oct 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants