CODEWITHSUNDEEP
javamultithreadingconcurrencysynchronized
user990246
user990246

Reputation:

Java application stuck when using synchronized keyword

I have a class that starts a few threads. Each thread (extends Thread) calls a new instance of class WH, class WH has a variable that is to be shared among all threads. So the hierarchy looks like:

class S extends Thread {
....
....
  WH n = new WH(args);
....
....
}

Now class WH has a variable that is to be shared, declared as:

private static volatile Integer size;

One of the functions tries to access size through Synchronized:
Synchronized (size) { // Program gets stuck at this line
 ... stuff ...
}

It gets stuck even if I spawn off just one thread. Any idea why this is happening? (FYI- I do not want to use AtomicInteger based on my design choices)

Thanks

Upvotes: 3

Views: 2606

Answers (2)

user177800
user177800

Reputation:

Your problem is that locking on a Non-Final variable reference has useless semantics.

Anytime you see something doing a synchronized(var); and var is an instance or static variable and isn't marked final, it is an error, because anything can come along and do a var = new Thing(); and now there are at least 2 threads that can operate on that block at the same time, this is a logical error no exceptions. Every Java lint style checker flags this as a critical error, just because the compiler doesn't catch this doesn't mean it has any usefulness in any case.

In this case, you are exposing these useless semantics by changing the value of the immutable Integer class.

Your Integer variable size is non-Final and is Immutable which means every time you change it you must change the reference to the new object that represents the new value and every thread will get a new and different reference to lock onto. Thus no locking.

Use a private static final AtomicInteger size = new AtomicInteger();

And then you can synchronize(size); since size is now final you can mutate it in place and get the intended and correct semantics.

or you can synchronize(some_other_final_reference); and use a regular int as long as that reference that is synchronized on is final and can be in scope of any thread that needs to acquire a handle to it, it will work.

Personally I would use the AtomicInteger it is more cohesive that way, you are locking on what you don't want changing by any other thread, self-documenting and clear intentions.

Upvotes: 2

sparc_spread
sparc_spread

Reputation: 10843

I cannot use AtomicInteger since I need to get the value of size, check a condition on it, and increment or not based on the condition. So I have to do a get then possibly an increment on it. I still need locking in that case.

I believe what you are describing is something that AtomicInteger can definitely do, without locking, via the compareAndSet() method, no? Though the only supported test is equality, so maybe that won't work for you.

Also, if you are planning on synchronizing on the variable, then there is no need to also make it volatile.

Upvotes: 1

Related Questions