Recursive/Multithreading Program Issues

I'm new to Java and multithreading. I have a following problem:

I have two classes named Class A and Class B running in two different threads.

Class A has the method onNewEvent().

Once that method is invoked, it will ask Class B to do some work. As soon as Class B finishes the work it invokes the method onJobDone() defined also in Class A.

Now, here comes the problem: what I want is to create a new job within the method onJobDone() and to send it again to Class B.

Here is what I do (pseudocode) in the s开发者_JAVA技巧equence of execution

A.onNewEvent(){
    //create job
    //ask B to do it
    B.do()
}

B.do{
    // Do some stuff
    A.jobDone()
}

A.onJobDOne(){
    B.do()   //doItAgain

    // print message "Thank you for doing it"
}

The problem is that the message "Thank you for doing it" never gets printed. In fact, when onJobDone() method is invoked, it invokes B.do() because B.do() is very fast. It invokes onJobDone() immediately, so execution flow never comes to the PRINT MESSAGE part of the code.

I suppose this is one of the nasty multithreading problems.

Any help would be appreciated.

This is not a mutli-threaded problem, you have just created an infinite loop. B.do calls A.onJobDone which calls B.do which calls A.onJobDone etc. So the execution will never reach the print message line. You need a break-out condition so that within onJobDone you can decide whether you want to 'doItAgain'. At some point you will decide not to do it again and at that point your code will reach the print message line.

It might help if you describe what it is you are trying to achieve and we could give you some pointers about the best way to implement it. I'm not sure if the way you are trying to solve your problem is really the best way.

Here's a suggestion

 A.onNewEvent(){
     //create job
    //ask B to do it
     B.do()

}

B.do{
    // Do some stuff
    A.onJobDone()
}

A.onJobDone(){
    // print message "Thank you for doing it" (B.do() has just completed, so it 
    //will print for every time you call B.do()
    if (shouldDoJobAgain()) //it should stop some time, right?
      B.do()

}

Invoking a class method isn't inherently tied to a thread. In other words, calling B.do() inside A.onNewEvent() in thread 1 will still execute in thread 1. In other other words, methods of an object can be invoked from any thread.

To have a method execute within a given thread, you'll need to call that method within the thread's run method. To support this, you'll need to define: methods on B that signal when there's a new job, a field of B to hold job information (in general, you'd use a queue, but a simple integer might work here) and properly synchronize access to the fields of B. It's been a little bit since I've done this in Java (there might be a race condition or deadlock somewhere), but I believe something like the following should work.

class A {
    ...
    public void onEvent() {
        b.addJob();
    }
    // careful about making onJobDone synchronized
    public void onJobDone() {
        // do it again
        b.addJob();
        ...
    }
}

class B extends Thread {
    A a;
    int jobCount;
    boolean work;

    ...
    public void run() {
        shouldWork(true);
        while (shouldWork()) {
            while (haveJobs()) {
                doingJob();
                ...
                didJob();
            }
            waitForWork();
        }
    }
    public synchronized void addJob() {
       ++jobCount;
       notify();
    }
    protected synchronized boolean haveJobs() {
        return jobCount > 0;
    }
    protected synchronized void doingJob() {
        /* could also decrement 'jobCount' in didWork(), in which case
           it will need to be made synchronized
         */
        --jobCount;
    }
    protected void didJob() {
        a.onJobDone();
    }
    protected synchronized void waitForWork() {
        while (! haveJobs()) {
            try {
                wait();
            } catch (Exception e) {
        }
    }
    public synchronized void shouldWork(boolean w) {
        work = w;
    }
    protected synchronized boolean shouldWork() {
        return work;
    }
}

The main problem to avoid is deadlock. Most of the synchronized methods won't cause deadlock. waitForWork calls wait, which releases the monitor so other threads can successfully call addJob.

Note that the above will still run forever, since the end of a job causes a new job to be scheduled. The advantages over how you've coded it is that A.onJobDone will finish and you won't get a stack overflow. If you want each A.onEvent to cause B to handle n jobs, define A as:

class A {
    int jobsPerEvent;
    int remainingJobs;
    ...
    public void onEvent() {
        synchronized (this) {
            remainingJobs += jobsPerEvent;
        }
        b.addJob();
    }
    public synchronized void jobsRemain() {
        return remainingJobs > 0;
    }
    public void onJobDone() {
        synchronized (this) {
            --remainingJobs;
        }
        // do it again
        if (jobsRemain()) {
            b.addJob();
        }
        ...
    }
}

Another approach is to implement asynchronous method calls, refactoring the asynchronous method (B.do) into a separate class.

class B {
    A a;
    Set jobs;
    class BAlgo extends Thread {
        public void run() {
            // stuff originally in B.do
            ...
            done();
            jobs.remove(this);
        }
    }

    public void do() {
        BAlgo algo = this.new BAlgo();
        algo.start();
        jobs.add(algo);
    }
    protected void done() {
        a.onJobDone();
    }
    ...
}

I'm sure refactoring the do method into a class that implements the algorithm is another pattern, but I'm not certain of its name. It's not quite the bridge pattern, nor is it the template method pattern. It's almost follows the proxy pattern, but not quite.