80

Let me use this small and simple sample:

class Sample {
    private String msg = null;

    public void newmsg(String x){
        msg = x;
    }

    public String getmsg(){
        String temp = msg;
        msg = null;
        return temp;
    }
}

Let's assume the function newmsg() is called by other threads that I don't have access to.

I want to use the synchonize method to guarantee that the string msg is only used by one function per time. In other words, function newmsg() cannot run at the same time as getmsg().

rjohnston
  • 7,153
  • 8
  • 30
  • 37
Winter
  • 1,896
  • 4
  • 32
  • 41
  • 4
    Are you asking how to use "synchronized" keyword in Java? A simple google search comes back with a lot of useful hits including this one http://download.oracle.com/javase/tutorial/essential/concurrency/syncmeth.html – Kal May 02 '11 at 20:05
  • 4
    Btw, it would be much better to call our getmsg() method something like popmsg() or consumemsg() since it modifies the class – naumcho May 02 '11 at 20:05

6 Answers6

184

That's pretty easy:

class Sample {
    private String message = null;
    private final Object lock = new Object();

    public void newMessage(String x) {
        synchronized (lock) {
            message = x;
        }
    }

    public String getMessage() {
        synchronized (lock) {
            String temp = message;
            message = null;
            return temp;
        }
    }
}

Note that I didn't either make the methods themselves synchronized or synchronize on this. I firmly believe that it's a good idea to only acquire locks on objects which only your code has access to, unless you're deliberately exposing the lock. It makes it a lot easier to reassure yourself that nothing else is going to acquire locks in a different order to your code, etc.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 11
    what is the difference between set the synchronize on function or create this lock ? – Winter May 02 '11 at 20:07
  • 1
    +1 Good and sensitive but beware of over-engineering. Maybe this class is just a message dispatcher so instantiating a lock object is just wasteful. Don't exaggerate. – gd1 May 02 '11 at 20:08
  • 6
    @Giacomo: I like my code to be readable, and designed so that I can reason about it. That's doubly important when multi-threading is involved. It's not exaggerating to say that this is the kind of code I'd almost *always* use. As for being *wasteful*... how often would you expect this waste to be significant? – Jon Skeet May 02 '11 at 20:09
  • @Jon: very little, indeed. However I too like my code to be readable and on my personal opinion (I might be wrong) using another field when it's not necessary (and we really don't know if it is) makes stuff less readable. However, good answer. – gd1 May 02 '11 at 20:10
  • 4
    @Giacomo: The point is that it's obvious that you're locking on something no-one else can. That seems reasonably straight-forward to reason about: you read the code, and you're done. Now consider your example: how do you know what *might* synchronize on your object? How can you understand the locking behaviour? You have to read *all* the code involved. I'd therefore say it's less readable if you're actually bothered by the behaviour being correct. Frankly I believe it was a mistake to allow *all* objects to be locked in Java. (A mistake .NET copied, unfortunately.) – Jon Skeet May 02 '11 at 20:14
  • 1
    What about the visibility on `message`? With `message` missing the `volatile` modifier, even with the atomic access on lock one thread may not see the changes the other thread(s) have made to `message`. – Stu Thompson Mar 11 '15 at 19:58
  • 1
    @StuThompson: I believe they will, because of the interaction `synchronized` has with the memory model. – Jon Skeet Mar 11 '15 at 22:04
  • @JonSkeet: Ah, ok. Had to go re-read the 133 FAQ...had forgotten about the monitor release memory writing, thread cache invalidating behaviors. Thanks. – Stu Thompson Mar 12 '15 at 19:50
  • Will this solution help prevent the owner thread of `Sample`'s object to modify msg if the intrinsic lock on `lock` variable defined here is obtained by another thread? – Akash Agarwal Feb 05 '16 at 07:39
  • @JonSkeet what I mean by that is the thread that initialized the object of `Sample`'s class which we want to modify. – Akash Agarwal Feb 05 '16 at 08:14
  • 1
    @AkashAggarwal: That thread has no special ownership of it - it would need to acquire the monitor associated with `lock` just like any other thread. – Jon Skeet Feb 05 '16 at 08:19
  • @JonSkeet: Thanks, I was told something else here in the first comment to my question and that's what raised the confusion. http://stackoverflow.com/questions/35217897/does-synchronize-keyword-prevent-a-thread-from-using-its-own-variable-while-lock – Akash Agarwal Feb 05 '16 at 08:27
  • 1
    @AkashAggarwal: No, I don't think you were given a different message. You need to differentiate between "ability to change state" (which just depends on whether or not your code has access to the field) and "ability to acquire a monitor". If the only way of changing state is via acquiring a monitor (as per my example) then you're at least closer to thread safety. Again, which thread created the object is irrelevant. – Jon Skeet Feb 05 '16 at 08:31
  • 4
    @Winter synchronized method blocks "this", so Sample object itself. But synchronized block on Object "lock" will block "lock", not the Sample – Jemshit Jul 26 '16 at 08:28
  • Why not just lock on the String instead of creating another object just to lock to? – Philip Rego Jun 08 '17 at 21:47
  • 4
    @PhilipRego: Firstly, because then the value being locked on would change, which makes things unsafe in all kinds of ways. Secondly, because it could easily mean locks being unintentionally shared between multiple objects, leading to all kinds of other problems. Thirdly: separation of concerns. I'm very keen on a field having one purpose. – Jon Skeet Jun 08 '17 at 21:56
  • @JonSkeet - Would it be the same behavior if one was to use `private volatile String message = null` and `public String getMessage() { return message ; }` ?. Are there any differences between two approaches ? Thank you in advance. – s7vr Jul 05 '19 at 13:49
  • @user2683814: Possibly, but I'd need to look *very, very* carefully at the spec to say for sure. I try not to state much about the precise semantics of `volatile` these days, as it's definitely "expert level" stuff. – Jon Skeet Jul 05 '19 at 13:53
60

For this functionality you are better off not using a lock at all. Try an AtomicReference.

public class Sample {
    private final AtomicReference<String> msg = new AtomicReference<String>();

    public void setMsg(String x) {
        msg.set(x);
    }

    public String getMsg() {
        return msg.getAndSet(null);
    }
}

No locks required and the code is simpler IMHO. In any case, it uses a standard construct which does what you want.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
  • 10
    FYI there are classes in `java.util.concurrent.atomic` for primitive types such as the `AtomicBoolean` and `AtomicInteger` – Hugh Jeffner Dec 21 '15 at 19:04
10

From Java 1.5 it's always a good Idea to consider java.util.concurrent package. They are the state of the art locking mechanism in java right now. The synchronize mechanism is more heavyweight that the java.util.concurrent classes.

The example would look something like this:

import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

public class Sample {

    private final Lock lock = new ReentrantLock();

    private String message = null;

    public void newmsg(String msg) {
        lock.lock();
        try {
            message = msg;
        } finally {
            lock.unlock();
        }
    }

    public String getmsg() {
        lock.lock();
        try {
            String temp = message;
            message = null;
            return temp;
        } finally {
            lock.unlock();
        }
    }
}
cpcolella
  • 247
  • 4
  • 10
Niclas Meier
  • 176
  • 2
  • 2
    per the [docs](https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/locks/ReentrantLock.html), the lock should be done prior to the try block. – Aaron Roller Jan 03 '17 at 01:49
5

Use the synchronized keyword.

class sample {
    private String msg=null;

    public synchronized void newmsg(String x){
        msg=x;
    }

    public synchronized string getmsg(){
        String temp=msg;
        msg=null;
        return msg;
    }
}

Using the synchronized keyword on the methods will require threads to obtain a lock on the instance of sample. Thus, if any one thread is in newmsg(), no other thread will be able to get a lock on the instance of sample, even if it were trying to invoke getmsg().

On the other hand, using synchronized methods can become a bottleneck if your methods perform long-running operations - all threads, even if they want to invoke other methods in that object that could be interleaved, will still have to wait.

IMO, in your simple example, it's ok to use synchronized methods since you actually have two methods that should not be interleaved. However, under different circumstances, it might make more sense to have a lock object to synchronize on, as shown in Joh Skeet's answer.

no.good.at.coding
  • 20,221
  • 2
  • 60
  • 51
3

In this simple example you can just put synchronized as a modifier after public in both method signatures.

More complex scenarios require other stuff.

gd1
  • 11,300
  • 7
  • 49
  • 88
2

If on another occasion you're synchronising a Collection rather than a String, perhaps you're be iterating over the collection and are worried about it mutating, Java 5 offers:

Ken
  • 30,811
  • 34
  • 116
  • 155