How can I ensure thread safety in the following context?

Question

I have a method called processOutbox. I want it to be thread safe. I do not want another thread to call this method while there was one thread on it. I implemented it as follows. Did I do it right? Are there any loopholes in my implementation? If they are, then please advise how I can resolve this.

this.start(); outboxLock.lock(); timer = new Timer(); try{ timer.scheduleAtFixedRate(new TimerTask() { public void run() { processOutbox(); } }, 0, period); } finally{ outboxLock.unlock(); } 

Answer 1

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

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

Additional information: https://docs.oracle.com/javase/tutorial/essential/concurrency/syncmeth.html

If your code has an instance of your CallClass, for example, myInstance , all calls to processOutbox() will be thread safe because they will be blocked at the instance level.

For instance:

 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 wait until thread1 completes the call to the processOutbox method

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 call the method in different instances.

Someone specifically asked about using ReentrantLock. Therefore, I am adding this answer to this, 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 something in which you keep other threads out of the block, without forcing them to block with tryLock.

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

Answer 2

Use CountDownLatch to sync.