-1

For homework, I am writing a stop() method for a server that managers multiple threads that perform transactions. Stop() is supposed to 1. submit a stop command to a queue of commands and 2. call join() on all the threads so that transactions have a chance to be completed.

However, join() is not working as expected. When I try to call join(), I get a NullPointerException. I modified the server thread to sit and wait a second after I submit stop commands, but that's a poor substitute when I should be using join().

public void stop() throws InterruptedException {
    // TODO Auto-generated method stub

    System.out.println("server stop()");
    for (CommandExecutionThread thread : threads) { 
        System.out.println(threads.length);
    }

    for (CommandExecutionThread thread : threads) { 

        Command e = new CommandStop();
        queue.add(e);

    }   

    for (CommandExecutionThread thread : threads) { 
        if (thread != null) 
        {
            thread.join(); 
        } 
        if (thread == null)
        {
            System.out.println("NULL thread"); //threads are always null at this point 
        }
    }

    for (CommandExecutionThread thread : threads) { 
        System.out.println(threads.length);
    }
    wait(1000);//simulate the wait that I would have had from join() 

}

CommandExecutionThreads poll() commands from the commandQueue, and when it encounters a stop command, it returns:

public void run() {
    while (true) { 
        synchronized (commandQueue) {
            while (commandQueue.isEmpty()) { 
                try {
                    commandQueue.wait();
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
            }//end while(commandQueue.isEmpty())
        }//end synchronized(commandQueue)

        Command c; 

        synchronized (commandQueue) { 

            c = commandQueue.poll(); 

            if (c.isStop()==true)
            { 
                System.out.println("\tSTOP");
                return;
            } 
            else 
            { 
                //System.out.println("\tTRANSACTION"); 
            } 

            if (executeCommandInsideMonitor) { 
                try {
                    c.execute(bank);
                } catch (InsufficientFundsException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }
            }

            commandQueue.notifyAll();
        } // end sync commandQueue 
    } //end while(true)
}//end run() 

I initialize my array of threads earlier when I create the bank server. The threads themselves pull commands off the queue when they become available. For diagnostic purposes, I've put a printout of thread.isAlive() and that works. By the time I'm in the stop method, thread.isAlive() throws a null pointer error :

public class BankServerImpl implements BankServer {
Queue<Command> queue = new LinkedList<Command>(); 
CommandExecutionThread[] threads; 
boolean executeCommandInsideMonitor; 
Bank bank; 

/**
 * Constructor for BankServerImpl, which implements BankServer
 * 
 * @param bank
 * @param serverThreads
 * @param executeCommandInsideMonitor
 */
public BankServerImpl(Bank bank, int serverThreads, boolean executeCommandInsideMonitor) {

    this.bank = bank; 
    this.executeCommandInsideMonitor = executeCommandInsideMonitor;
    threads = new CommandExecutionThread[serverThreads]; 

    for(CommandExecutionThread thread : threads) { 
        thread = new CommandExecutionThread(bank, queue, executeCommandInsideMonitor);
        thread.start(); 
        System.out.println(thread.isAlive());   

    }
}
L Wang
  • 19
  • 5
  • 5
    Where is `threads` declared, and what writes values into it? If it contains nulls, that's likely because you're simply never initializing it. – Daniel Pryden May 05 '15 at 18:22
  • 1
    `if (thread == null) { System.out.println("NULL thread"); //threads are always null at this point }` there is no code shown above that initialize threads array, Probably it is never initialized, check your code where you doing that. – kr.pradeep May 05 '15 at 18:23
  • 4
    You are putting nulls *into* your thread collection/array. Still null when you get it out again. – user207421 May 05 '15 at 18:24
  • If the threads are `null` most likely you haven't initialised the values of the array. – Peter Lawrey May 05 '15 at 18:37
  • I believe I initialize my threads within my array during the constructor. I've updated the original post to include that code. – L Wang May 05 '15 at 18:53

1 Answers1

2

When you create your threads, you never insert them into the array.

for(CommandExecutionThread thread : threads) { 
    thread = new CommandExecutionThread(bank, queue, executeCommandInsideMonitor);
    thread.start(); 
    System.out.println(thread.isAlive());   
}

This code does not cause the array to be updated. You need something like this:

for (int i = 0; i < serverThreads; i += 1) {
    CommandExecutionThread newThread = new CommandExecutionThread(bank, queue, executeCommandInsideMonitor);
    threads[i] = newThread;
    newThread.start();
    System.out.println(newThread.isAlive()); 
}
pens-fan-69
  • 979
  • 7
  • 11
  • FYI, you would be much better off using an ArrayList. These sorts of things are much easier and cleaner using collections. – pens-fan-69 May 05 '15 at 19:11
  • Thank you! It wasn't clicking for me until I saw the side-by-side comparison of the for loops. This makes complete sense now... thank you so much for the solution, and the tip for using collections. – L Wang May 05 '15 at 19:19
  • No problem @LWang. BTW, if you consider this an acceptable answer, I would appreciate it if you marked it as answering your question. – pens-fan-69 May 05 '15 at 19:22
  • Done! This was my first ever post to StackOverflow, so thank you for answering my question so quickly and helping me learn the proper etiquette. You have my green check! – L Wang May 06 '15 at 01:25
  • No problem. I think you'll find that there are a lot of people out there willing to help. Don't let the one down vote discourage you. – pens-fan-69 May 06 '15 at 16:26