1

I have a controller class as follows:

public class Controller {
    private Driver driver;

    public void method1() {/**/}
    public void method2() {/**/}
    public void method3() {/**/}
}

Each method uses the driver to perform some action. method2 however is a longer-running process and requires a lock on the driver, because if the other methods use the driver while method2 is running, then errors will occur.

Now the way I am currently set up, I have implemented a ReentrantLock with fair scheduling in the controller's calling class which takes care of thread safety as follows:

public class Caller {
    private static final Lock LOCK = new ReentrantLock(true);

    private Controller controller;

    public void startStuff() { // Runs in multiple threads.
        ...
        ...
        LOCK.lock();
        try {
            controller.method1();
        } finally {
            LOCK.unlock();
        }
        ...
        ...
        LOCK.lock();
        try {
            controller.method2();
        } finally {
            LOCK.unlock();
        }
   }
}

I would instead like to make the Controller class thread safe instead of having any calling classes deal with it. The way I assume this is to be done is by implementing the try-finally block at the beginning and end of each method, but I have around 15 methods in the class. Is this really the way to do it? It seems like a tonne of boilerplate code:

public class Controller {
    private static final Lock LOCK = new ReentrantLock(true);
    private Driver driver;

    public void method1() {
       LOCK.lock();
       try {
           /**/
       } finally {
           LOCK.unlock();
       }
    }
    public void method2() {
       LOCK.lock();
       try {
           /**/
       } finally {
           LOCK.unlock();
       }
    }
    public void method3() {
       LOCK.lock();
       try {
           /**/
       } finally {
           LOCK.unlock();
       }
    }
}

I cannot just add synchronized to each method because I require fair scheduling for the application to work, and using a ReentrantLock works perfectly.

How exactly do I make the Controller class thread-safe using a ReentrantLock?

Cristian
  • 6,765
  • 7
  • 43
  • 64
  • 1
    Failing AOP (or something similar), your suggestion is the proper implementation. – Sotirios Delimanolis Jan 02 '15 at 15:37
  • Your implementation looks ok... Remember to lock just before driver code and release after you are done with operation on driver. But remember this will be an overhead if driver does many things apart from executing critical section. – SMA Jan 02 '15 at 15:39
  • "...It seems like a tonne of boilerplate code..." Yep! That's Java in a nutshell. (In a different programming language, we might have solved this problem with a macro.) – Solomon Slow Jan 02 '15 at 16:13

1 Answers1

4

You could wrap your locking code up into a block and pass the actual code implementation to it so that you remove the duplication/boilerplate. Something like the following (Java 7):

public void method1() {
   withLock(new LockableMethod() {
       public void run() {
           /** your method1 code here **/
       }
   });
}

private void withLock(LockableMethod lockableMethod) {  
   LOCK.lock();
   try {
       lockableMethod.run();
   } finally {
       LOCK.unlock();
   }
}

If you're using Java 8 you can remove even more boilerplate by just passing in a Lambda:

public void method1() {
   withLock(() -> { 
           /** your method1 code here **/
       }
   );
}

private void withLock(Consumer<T> consumer) {
    LOCK.lock();
    try {
        consumer.accept();
    } finally {
        LOCK.unlock();
    }
}
tddmonkey
  • 20,798
  • 10
  • 58
  • 67
  • 1
    Or you could make it even shorter with a `Closeable` wrapper, such as http://stackoverflow.com/a/11000458/212870. – Alan Stokes Jan 02 '15 at 15:50