• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Synchronization on a non-final field.

 
Ranch Hand
Posts: 149
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi

Intellij shows a warning every time I synchronize on a non-final class field. Here is an example:



This inspection reports instances of synchronized statements where the lock expression is a non-final field. Such statements are unlikely to have useful semantics, as different threads may be locking on different objects even when operating on the same object.



I don't fully understand what does it mean. I know that object o will be set only once and before method x() is invoked. Is there still a problem with such code ?
 
(instanceof Sidekick)
Posts: 8791
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
For a number of threads to synchronize a block of code correctly they need to synchronize on the same object instance. That sample code would allow someone to change "o" to a new object instance between the time Thread 1 synchronizes and the time Thread 2 synchronizes. Then both threads could enter the synchronized block at the "same time". We could probably make up a scenario where this is correct behavior, but it seems a lot more likely that it is not what we would like.

Did that makes sense?
 
Maris Orbidans
Ranch Hand
Posts: 149
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Yes, that's how I understand this issue too. But in my case I am sure that nobody will change object reference o ever after it has been set. So I guess I am on the safe side.
 
Wanderer
Posts: 18671
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
There's a problem with that though - it's still possible for another thread to think that o == null after setO() has been called. That's the sort of weird thing that happens with threads if they're not properly synchronized. And in this case the synchronization only occurs after o has been accessed, which is to late to ensure that o is accessed correctly. You could fix this by making o transient, though that's bit unusual for something that isn't supposed to change. Or you could declare both setO() and x() to be synchronized - that is, synchronized on the current instance of the X class, not the o variable. In that way, you would synchronize before accessing o, and ensure that you got the correct o. For that matter, if you synchronize on the X instance, do you still need to sync on o at all? You may not need to anymore.

All in all, making o final is probably the simplest for you to do, if that's possible. We can't guess what these methods are really doing, so it's hard to offer good suggestions about what sort of redesign would make the most sense here.
 
Ranch Hand
Posts: 92
Android Eclipse IDE Suse
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Maris,

Yes, that's how I understand this issue too. But in my case I am sure that nobody will change object reference o ever after it has been set. So I guess I am on the safe side.



Maybe you know this, but you don't have to think only to yourself. Since your method setO(Object o) is public, what does prevents another developer that uses your class to call it and change the object you are synchronizing on?

If you would have declared your instance field final, the compiler would have been forced you to initialize it in the constructor, that would have made your IDE happy I guess.
 
Ranch Hand
Posts: 1170
Hibernate Eclipse IDE Ubuntu
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

Originally posted by Maris Orbidans:
Yes, that's how I understand this issue too. But in my case I am sure that nobody will change object reference o ever after it has been set. So I guess I am on the safe side.



If this is the case then try setting its value in the constructor and/or using a factory method to set it.
 
Ranch Hand
Posts: 443
3
Eclipse IDE C++ Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Jim presumably you mean 'transient' in its memory sense of the word rather than as Java defines it, i.e. aren't you describing volatile, or am I amount to learn some new property of transient ;-)

PS

Yes, that's how I understand this issue too. But in my case I am sure that nobody will change object reference o ever after it has been set. So I guess I am on the safe side.



Obviously currently you'ed have the implied condition that you object was created in and your setter called within the same thread.
 
Jim Yingst
Wanderer
Posts: 18671
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
[Chris]: Jim presumably you mean 'transient' in its memory sense of the word rather than as Java defines it, i.e. aren't you describing volatile, or am I amount to learn some new property of transient ;-)

Yes, I have an annoying tendency to say transient when I mean volatile, and vice versa.
 
Greenhorn
Posts: 14
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Slight dig here, but I found this thread by Googling this exact message, so I'm going to assume others will as well.

I have ran into this same message, and I've replaced the code as such:


I was wondering A, is this okay in general, and B, would this be bad because I am getting the object twice instead of acquiring a lock then using the locked object, IE:


JSYK, I am also setting o via a synchronized mutator.

Both of these make the warnings go away, and both of them seem to be doing what I want them to, but I am wondering which is proper and why, and whether or not there will be consequences and why.


Thanks!
 
Ranch Hand
Posts: 43
Netbeans IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator

In the first case,the object used to lock is private & there is no way for other any other thread to change it (assuming, there is no public setter methods provided). Hence, there are no warnings from the IDE.

In the second case, you are locking on local variable. There is no way for any other thread to access the local variable of another thread.Hence, there are no warnings from IDE.

The best/suggested way to lock in the given scenario is this:


Hope, this helps.
 
Consider Paul's rocket mass heater.
reply
    Bookmark Topic Watch Topic
  • New Topic