views:

121

answers:

4
A: 

Consider using an implementation of ExecutorService. Add tasks using the submit() methods, then tell the service not to accept more tasks with the shutdown() method -- tasks that have already been accepted will complete normally.

Drew Wills
Interesting idea. We have a lot of ExecutorServices in our code, but hadn't thought of using it here. The biggest challenge I see is that Thread X is really an HTTP request (running through Jetty, proxied though Apache HttpClient), and when Y gets called I need to abort() the active HttpClient request. Perhaps I should have added that to my requirements: when Y is called, it must call some custom shutdown code for X.
Patrick Lightbody
+3  A: 

Looks like a task for ReentrantReadWriteLock. Something like this:

class A {
    private final ReadWriteLock lock = new ReentrantReadWriteLock();

    public void x() {
        if (!lock.readLock().tryLock()) throw new RuntimeException();
        try {
            ...
        } finally {
            lock.readLock().unlock();
        }
    }

    public void y() {
        lock.writeLock().lock();
        try {
            ...
        } finally {
            lock.writeLock().unlock();
        }
    }
}
axtavt
I looked at doing this as well, but I end up with the similar issue I commented on in Drew's answer: I need y() to, before lock(), abort all calls to x() so that they finish up "quickly" and we're not waiting on lock() for very long in y().Your code example isn't exactly like mine, since x() and y() are in different classes. I think somehow I need another lock to be able to safely get a reference to the X objects in to something that Y object can call to shutdown...
Patrick Lightbody
@axtavt: This looks great. +1. However, I think we probably want to make it a fair lock (`new ReentrantReadWriteLock(true);`). If not, some thread can sneak execute x() even if already y() has been called. (btw fairness will greatly hurt performance)
Enno Shioji
A: 

Ok, how about using an ActiveObject?

class X {
    private static final ExecutorService service = Executors.newCachedThreadPool();


    public void x(){
       //Using anonymous class for brevity. Normal Runnable is ok. May be static inner class.  
       service.submit(new Runnable(){
                             @Override
                             public void run(){
                                   //do stuff
                             }
                       });
    }

    //Derived from Java6 ExecutorService API        
    public void shutDownAndAwait(){
        service.shutdownNow();
        try{
           if (!service.awaitTermination(60, TimeUnit.SECONDS))
              throw new TerminationException("Service did not terminate");
           }
        } catch (InterruptedException ie) {
           // (Re-)Cancel if current thread also interrupted
           service.shutdownNow();
           // Preserve interrupt status
           Thread.currentThread().interrupt();
        }
   }//shutdownAndAwait

}//X


class Y {

    private final X x = //inject reference

    public void y(){
        x.shutdownAndAwait();
    }
}

The problem is that now execution of x() is asynchronous, so the original thread that called the x() are not sure when it was completed. And they are not interrupted when y() is called...

Enno Shioji
+2  A: 

Java 7 is comming out with a new concurrency construct class, java.util.concurrent.Phaser. The phaser is a glorified cylcic barrier that allows awaiting on specific phases (or iterations) of a concurrent execution. You can download the latest binary jar at JSR 166's Interest Site.

My thought here is you have a volatile boolean yHasEnetered flag that initializes default to false. So in your X execution you could do :

        if(yHasEnetered)
            throw new IllegalStateException();
        phaser.register();
        //do work here
        phasre.arrive();

The phaser itself will keep a running count of all registered parties so that when Y thread has entered, it can set the flag accordingly, register itself and await advance.

        yHasEntered=true;
        int phase = phaser.register();
        phaser.arriveAndAwaitAdvance(phase);
        //do y work here
        yHasEntered=false;

At this point for Y thread. It will register itself get the phase that the phaser is currently on, arrive and wait for all other threads executing to reach their respective arrive block.

John V.
@John Very interesting! I'm checking in to Phaser a bit more, but it looks right on the money.
Patrick Lightbody
I wish I knew Phaser enough to explain how to achieve what Im about to say next. The phaser, like I mentioned, will keep track of phases. So what you can do next, is when yHasEnetered evaluates to true, instead of throwing an exception, you can have all X threads wait on the next phase to occur. The next phase would be incremented when Y thread is finished on a theoretical level.
John V.