how to share a variable between two threads

Question

I just inherited some code, two threads within this code need to perform a system task. One thread should do the system task before the other thread. They should not be performing the system task together. The two threads do not have references to each other.

Now, I know I can use some sort of a semaphore to achieve this. But my question is what is the right way to get both threads to access this semaphore.

I could create a static variable/method a new class :

public class SharedSemaphore
{
    private static Semaphore s = new Semaphore (1, true);

    public static void acquire () {
        s.acquire();
    }

    public static void release () {
        s.release();
    }
}

This would work (right?) but this doesn't seem like the right thing to do. Because, the threads now have access to a semaphore, without ever having a reference to it. This sort of thing doesn't seem like a good programming practice. Am I wrong?

UPDATE:

I renamed the two methods performTask to acquire, and the other one to release, because I felt that was distracting from the actual question.

Answer 1

I think, you can make performSystemTask method synchronized and it'll be enough.

Answer 2

If the threads should always run one after the other, they need a manager. I would wrap them into another class, even another thread class.

public class wrapperThread extends Thread {
    public void run() {
        Worker1Thread myThread = new Worker1Thread();
        myThread.start();
        myThread.join();
        Worker2Thread myThread = new Worker2Thread();
        myThread.start();
        myThread.join();
    }
}

This is the safest and cleanest way that I can think of.

If they simply should never run at the same time, then they should probably be the same thread.

Answer 3

I see that you made your Semaphore fair. So I guess you care about the order these "system tasks" are executed? Then, relying on the ordered arrival of threads is very fragile and dangerous in my opinion. This problem will also be present if you use synchronized keyword.

I'd say you should use CountdownLatch instead.

class TaskOrganizer {
    private final CountdownLatch firstTask = new CountdownLatch(1);

    public void firstTaskIsDone(){
        firstTask.countDown();
    }

    public void permissionForSecondaryTask(){
        firstTask.await();
    }
}

If you can't pass TaskOrganizer objects to your threads, then I guess making it static is okay, but generally it's better to pass instances to your threads (well, to the Runnables to be exact). You never know if you are going to need 2 TaskOrganizers. If you had used static, then things won't be as cleaner as it could have been.

I guess it's obvious, but one thread calls firstTaskIsDone(), and the other one blocks until it is done, calling permissionForSecondaryTask();. If you have tons of task to organize with tons of threads, you might want to roll out Phaser (scheduled to appear in JDK 7, backport available at http://gee.cs.oswego.edu/dl/concurrency-interest/).