java semaphore idiom

Question

I have a thread which does an action only when it gets exclusive access to 2 semaphores.

public void run(){
            boolean a1=false;
            boolean a2=false;
            boolean a3=false;
            while(true){
                try{
                    if(res[1].tryAcquire()==true){
                        a1=true;
                        if((res[2].tryAcquire()==true){
                            a2=true;
                            if(res[3].tryAcquire()==true)){
                                a3=true;
                                System.out.println("Rolled the tobacco");
                            }
                        }   
                    }
                }
                finally{
                    if(a1){
                        a1=false;
                        res[1].release();
                    }
                    if(a2){
                        a2=false;
                        res[2].release();
                    }
                    if(a3){
                        a3=false;
                        res[3].release();
                    }
                }
            }
        }
    }

Is there a better way to write this to make sure we do not upset the semaphore acquired count? Is there a way to check if a semaphore is acquired by the current thread?

Answer 1

In Java 7 a try with Closeable is possible. There certainly must be nicer solutions.

public class Region implements Closeable {

    private final Semaphore semaphore;

    public Region(Semaphore semaphore) {
        this.semaphore = semaphore;
        if (!semaphore.tryAcquire()) {
            throw NotAcquiredException(semaphore);
        }
    }

    @Override
    public void close() {
         semaphore.release();
    }
}

public class NotAcquiredException extends Exception { ... }

Usage:

    public void run() {
        boolean a1 = false;
        boolean a2 = false;
        boolean a3 = false;
        while (true) {
            try (Closeable r1 = new Region(res[1])) {
                 a1 = true;
                 try (Closeable r2 = new Region(res[2])) {
                     a2 = true;
                     try (Closeable r3 = new Region(res[3])) {
                          a3 = true;
                          System.out.println("Rolled the tobacco");
                     } catch (IOException e) {
                     }
                 } catch (IOException e) {
                 }
            } catch (IOException e) {
            }
       }

Answer 2

You could separate each acquire into a try...finally, not shorter, but gets rid of some variables and makes it fairly obvious what should happen for each lock. (I changed the array to zero based)

public void run(){
    while(true){
        if(res[0].tryAcquire()){
            try {
                if(res[1].tryAcquire()) {
                    try {
                    if(res[2].tryAcquire()){
                        try {
                            System.out.println("Rolled the tobacco");
                        } finally {
                            res[3].release();
                        }
                    }
                    } finally {
                        res[2].release();
                    }                               
                }
            } finally{
                res[1].release();
            }
        }
    }
}

If you need to acquire a lot of locks or do this in several places, then maybe a helper class would be nice. At least hides the boilerplate code of acquire and releasing the semaphores.

public void run() {
    SemaphoreHelper semaphoreHelper = new SemaphoreHelper(res);
    while (true) {
        try {
            if (semaphoreHelper.aquireAll()) {
                System.out.println("Rolled the tobacco");
            }
        } finally {
            semaphoreHelper.releaseAquired();
        }
    }
}

private static class SemaphoreHelper {

    private final Semaphore[] semaphores;
    private int semaphoreIndex;

    public SemaphoreHelper(Semaphore[] semaphores) {
        this.semaphores = semaphores;
    }

    public void releaseAquired() {
        while (semaphoreIndex > 0) {
            semaphoreIndex--;
            semaphores[semaphoreIndex].release();
        }
    }

    public boolean aquireAll() {
        while (semaphoreIndex < semaphores.length) {
            if (!semaphores[semaphoreIndex].tryAcquire()) {
                return false;
            }
            semaphoreIndex++;
        }
        return true;
    }
}