Can i mutate a lock object?

[yipking]

Is there any problem with the following code? Someone told me the getter synchronization guard won't work but I don't know why.

public class MyClass  {
 private volatile JobStatus status  = new JobStatus();
 protected void setJoblStatus (...) {
 synchronized (status) {
               status.set(…); //mutating status
 }
 }
 public JobStatus getStatus()  {
 synchronized (status){
 return status;
 }
 }
}

[sunde887]

Looks fine to me, I am far from an expert on concurrency, but client side locking should work assuming the status item doesn't use a private lock.

What was there reason for why the lock on getting the object wouldn't work? It may be acceptable to simply make the entire method synchronized(which synchronizes on "this")

You may want to make the instance variable final, you are never changing the actual object, just the object your are referencing, when possible, make variables final.

[kjkrum]

public JobStatus getStatus()  {
 synchronized (status){
 return status;
 }

This doesn't really do anything. What are you trying to accomplish?

[kjkrum]

Correction to my correction: I was wrong... the synchronization in that getter does absolutely nothing except slow down your program by a negligible amount.

[sunde887]

Without synchronization, isn't it possible to view the object in a stale state?

One thread begins changing the state of the object, then another thread gets the reference to the object, then the first thread finishes making a change.

I'm just asking for my own understanding. I'm understanding concurrency, but it's quite a bit more difficult than anything else I've seen in the language.

[kjkrum]

Yes, reading the fields of an object while holding the lock on that object ensures that the values are not stale... assuming the writes were also synchronized.

But in that getter, no fields of the object referenced by "status" are read or written in the synchronized block, so there's no point to the synchronization.

If you wanted to ensure that the reference being returned is fresh, you would synchronize on the object that contains the reference (i.e., "this"), not on the object that the reference refers to.

[yipking]

Thanks Guys! I got it!