4

I have a method called processOutbox. I want it to be thread safe. I don't want another thread to call this method while one thread is at it. I have implemented it the following way. Have I done it correctly? Are there any loopholes in my implementation? If there are any, then please advice on how I can resolve it.

this.start();
    outboxLock.lock();
    timer = new Timer();
    try{
    timer.scheduleAtFixedRate(new TimerTask() {
            public void run() {
               processOutbox();
            }
        }, 0, period);
    } finally{
        outboxLock.unlock();
    }
AnOldSoul
  • 4,017
  • 12
  • 57
  • 118

2 Answers2

5

If you want to make your method processOutbox, you should use the keyword synchronized:

public class YourClass{
    public synchronized void processOutbox(){
         //do all you want
    }
}

More info at:https://docs.oracle.com/javase/tutorial/essential/concurrency/syncmeth.html

If in your code you have an instance of YourClass called for example myInstance , all calls to processOutbox() will be thread safe because they will be locked at instance level.

For example:

YourClass myInstance = new YourClass();
 Thread thread1 = new Thread(){
    public void run(){
      myInstance.processOutbox();
    }
  }
 Thread thread2 = new Thread(){
    public void run(){
       myInstance.processOutbox();
    }
  }
thread1.start();
thread2.start();

Here thead2 will be waiting until thread1 finishes the call to "processOutbox"

But for example:

YourClass myInstance = new YourClass();
YourClass myInstance2= new YourClass();
Thread thread1 = new Thread(){
    @Override
    public void run(){
        myInstance.processOutbox();
    }
};
Thread thread2 = new Thread(){
    @Override
    public void run(){
        myInstance2.processOutbox();
    }
}
thread1.start();
thread2.start();

thead2 will NOT wait because they are calling the method on different instances.

Someone specifically asked about using ReentrantLock -- So I'm adding that response on to this one, because this one is correct.

public class YourClass {
    private Lock outboxLock = new ReentrantLock();
    public void processOutbox() {
        outboxLock.lock()
        try {
            // do stuff
        } finally {
            outboxLock.unlock()
        }
    }
}

I mention this specifically because you can also do things, where you keep other threads out of the lock without causing them to block by using tryLock instead.

public class YourClass {
    private Lock outboxLock = new ReentrantLock();
    public void processOutbox() {
        if( outboxLock.tryLock() ) {  
            try {
                // do stuff
            } finally {
                outboxLock.unlock()
            }
        }
    }
}
Vogel612
  • 5,620
  • 5
  • 48
  • 73
David Herrero
  • 704
  • 5
  • 17
  • **I don't want another thread to call this method while one thread is at it.** – Naman Gala Sep 22 '15 at 07:05
  • Java API: "When one thread is executing a synchronized method for an object, all other threads that invoke synchronized methods for the same object block (suspend execution) until the first thread is done with the object." – David Herrero Sep 22 '15 at 07:07
  • How can I achieve the same using reentrantlock? please advice goku :) – AnOldSoul Sep 22 '15 at 07:07
  • If you read it carefully, there it is mention **for an object** and not **for all object**. – Naman Gala Sep 22 '15 at 07:09
  • @mayooran - Lock and unlock at the beginning and end of `processOutBox()` respectively. Although *synchornizing* would have the same effect. So, yes, this answer is also correct – TheLostMind Sep 22 '15 at 07:10
  • I dont undestand you : " If you read it carefully, there it is mention for an object and not for all object" – David Herrero Sep 22 '15 at 07:12
  • @TheLostMind If another object/process tries to access the same method, will it allow to access the method? If so how can I not allow other objects to access as well? – AnOldSoul Sep 22 '15 at 07:12
  • @mayooran - In your current case it is allowing. If you mark the method as synchronized, then it wont. – TheLostMind Sep 22 '15 at 07:13
  • If another object/process tries to access the same method" of the same instace", it will not allow to execute until others threads release the lock. – David Herrero Sep 22 '15 at 07:14
  • If I understand the question of the OP, he is asking for exclusive lock for the method. No other object can access the method. – Naman Gala Sep 22 '15 at 07:14
  • @NamanGala - And *synchronized* prevents other **threads** from accessing that part of code. It's all about which part of code a thread can access. Locks are just gate keepers – TheLostMind Sep 22 '15 at 07:16
  • I might be wrong, but according to my understanding lock is applied on `this` which is the calling object. If same object is used to call the method from different threads then it will work as expected. And as the calling object gets changed, it will allow execution of the method. – Naman Gala Sep 22 '15 at 07:19
  • @TheLostMind So I have removed the implementation in my question. I just made my method synchronized. Shall I accept this answer as the answer? – AnOldSoul Sep 22 '15 at 07:20
  • I have edited my answer..maybe it will help a bit more now. – David Herrero Sep 22 '15 at 07:20
  • @DavidHerrero what would happen if an object called myInstance2 of "YourClass" tries to access the processOutbox method? – AnOldSoul Sep 22 '15 at 07:22
  • 1
    @NamanGala - Assuming `processOutbox()` to be an *instance level* method. See, using `synchronized` at method level is same as using `synchronized(this) {..}`. The lock will be on the current object. If two threads use 2 different objects to access this method, *the state of those 2 objects will be different*. Thus there will be 2 different locks. Which is fine. – TheLostMind Sep 22 '15 at 07:24
  • @mayooran - Objects dont call methods *threads do*. Threads call / execute methods on objects – TheLostMind Sep 22 '15 at 07:25
  • @mayooran - This will prevent other threads from executing *synchronized* code if their lock is also on the current object – TheLostMind Sep 22 '15 at 07:27
  • @TheLostMind thumps up for your explanation. But I guess @ mayooran wants static synchronized method. @ mayooran do you want only one object in your system to call your method at a time or is it ok if different objects of same class call the method at the same time? – Naman Gala Sep 22 '15 at 07:27
  • @NamanGala if he wants **Class** level synchronication instead of **Instance** level syncrhonization he justa have to change the declaration of `processOutBox` to `public static synchronized void processOutbox()` – David Herrero Sep 22 '15 at 07:29
  • @NamanGala - I can't be sure of that unless I see the complete flow and state changes done by that done – TheLostMind Sep 22 '15 at 07:29
  • @NamanGala I added a bit about using tryLock which will cause other threads to leave without processing, but not block, which is what I _think_ you actually want. – lscoughlin Sep 22 '15 at 13:03
1

Use a CountDownLatch for synchronization.

Gayan Jayasingha
  • 752
  • 2
  • 17
  • 33