0

I have a multithreading program, and I am wondering which of the way using "synchronized" is correct.

Way 1 : I have a class object (which will be passed to multiple threads), called MyClass, inside it:

public synchronized void set(String name) {
    this.name = name;
}

Way 2 : I have same class, but not having "synchronized" in its set method:

public void set(String name) {
    this.name = name;
}

And the caller will do this :

MyClass myclass = new MyClass();
synchronized(myclass) {
    myclass.set("myclass");
}

Can anyone tell me which one is correct way of implementing multithreading object sharing? I am confused by these two and I tried both, they seems to work correctly.

BenMorel
  • 34,448
  • 50
  • 182
  • 322
Ghost_000_cs
  • 133
  • 1
  • 8

6 Answers6

6

Synchronize on the method:

  • You do it in one place,
  • Callers do not have to worry about synchronization,
  • You don't duplicate the code wherever you need to call
  • Most importantly - if callers omit the synchronize, this will not work

It depends on the context, really - e.g. in some cases it's better for callers to do the synchronization, as then they can decide whether the synchronization overhead is worth it. For example, all callers that are known to work on one thread do not need synchronization and this will only slow things down.

For all things that are not time-critical, it will most likely be better to avoid issues you need to debug while on your 5th coffee at 3am, while your colleagues wave a baseball bat and yell words that are not to be mentioned to kids...

icyrock.com
  • 27,952
  • 4
  • 66
  • 85
  • 1
    I agree with this. To write a thread-safe program, the essence of decision is the "context" of your design. "The design" is about the proper choice from multiple tactics. – Mike Lue Mar 21 '12 at 02:44
1

The first option is the correct version in general.

If you had two separate classes calling MyClass.set() from separate threads there's no guarantee they will lock on the instance before calling set(). Technically from your example provided they are the same thing being that you synchronized on the instance you're calling set() on. However, the 2nd doesn't guarantee the client will always do that, and two if they don't it's unsafe.

Now technically in the example you provided there is no way that two threads could get into trouble because if one thread creates the instance and calls set(). There's no other way a thread could have access to that instance. So there will never be any lock contention. Just so you are aware how object creation influences multi-threaded programs. Remember two threads have to SHARE a common reference to an instance they intend to modify for any threading issues to be a problem. If they never share a common reference there is no safety concerns.

chubbsondubs
  • 37,646
  • 24
  • 106
  • 138
0

Usually you put the synchronized in the same place as the state you want to protect. So if MyClass stores a name you want to synchronize access to, MyClass should be synchronized.

jjjt78
  • 13
  • 3
0

A synchronized method guarantees that all attempts to use that method will be properly synchronized, using the object's base lock. This means the caller can't accidentally forget to lock the object- it's implicit in the function call.

Lock the method. Remember that you need to synchronize your get method as well, or it may reflect an inconsistent state of the object because, if it is not synchronized, it will not check the lock and can complete in parallel while the object's lock is held on another thread.

Adam Norberg
  • 3,028
  • 17
  • 22
0

If you go for synchronized block, it will lock a specific object. If you go for synchronized method, it will lock all the objects. Please refer below link, may be you will get your answer..

Is there an advantage to use a Synchronized Method instead of a Synchronized Block?

Community
  • 1
  • 1
kandarp
  • 4,979
  • 11
  • 34
  • 43
0

I'm going to vote for the second option. Classes in general should not be thread-safe (and so not be internally synchronized). The safety costs time and is often not needed. Further, you often get situations like: if (list.isEmpty()) list.add( filler );. If both methods are synchronized internally it does no good. The list can be empty on the first call and have 1,000,000 entries when the add call is made. The whole statement must be synchronized to be useful.

More generally, you need to decide, for each class, whether it is thread-safe or not. The ones that aren't will be faster and can be made thread-safe by the caller who, as in the above instance, may need some flexibility in how they go about it. Also, there are clever ways to avoid any need for synchronization at all, such as referencing a class from a single thread. Most Java Collections Framework classes are not and should not be thread-safe. Thread-safety should generally be handled at a high level rather than a low level.

Every once in a while you get a low level class that has to handle a lot of thread traffic, and then you do need a class that is thread-safe. My favorite example is JavaConcurrentSkipListMap, which does the same job as TreeMap. I use TreeMap 100 times for every time I use JCSLM, but when I do need it it's a lifesaver. (Without it, I'd have stuck with Java 1.4.)

If you are going to make a class thread-safe, do something besides synchronizing methods. You can end up blocking other threads when you don't need to. Someone else may synchronize on your class and so block methods that don't need to be blocked. At the very least synchronize on an internal object that exists purely for synchronization. Better still, synch on logical groups of fields within your class, and only place synch blocks where you need them.

There are other ways to be thread-safe without synchronization--immutable objects, compare-and-swap, etc. Also, firing up other threads so the main thread is not blocked can avoid critical bottlenecks, even if it adds to your machine's overall workload.

That said, I have had good luck, on rare occasion, just synchronizing every method in a class. And I can (intentionally) lock the whole thing just by synching on the class. It's simple and safe, and does no harm if the class is lightly used. But I sometimes guess wrong about class usage and have to give up on it when my program hangs, using only 2% of CPU despite 4 hyperthreaded cores.

I should perhaps add that most of my work has been with time critical software. But too much synchronization, even without actual deadlocks, can make any program run way too slow for any purpose. And if time doesn't matter, you should not be using multiple threads.

RalphChapin
  • 3,108
  • 16
  • 18