Yabo9797
Yabo9797

Reputation: 53

Java Threading: Futures only using results from first and last thread

I have a simple utility which pings a set of nodes and returns an ArrayList of strings to a future object to be outputted to a file. The program should run until terminated by the user.

It doesn't appear that the future receives the results (or at least passes them to the method to output to the file). No matter the number of threads I have concurrently running (always less than 100, determined by an input file), I am only outputting the results from the first and last initialized threads.

As a sanity check, I created a global variable in which each thread will send its results before closing and returning its results to the Future object. This variable is correctly updated by all threads.

Does anyone have any ideas why Future doesn't seem to be receiving all my results from the threads?

public class PingUtility{
    public static ExecutorService pool = Executors.newFixedThreadPool(100);
    static Future<ArrayList<String>> future;

    public static void main(String[] args) throws Exception {

        Timer timer = new Timer();
        TimerTask task = new TimerTask(){
            public void run(){
                //Creates a pool of threads to be executed
                ArrayList<String[]> nodes = new ArrayList<String[]>()
                future = pool.submit(new PingNode(nodes));
                }   
            }
        };

        timer.scheduleAtFixedRate(task, 0, interval);

        while(true){
            try{
                ArrayList<String[]> tempOutputArray = future.get();
                Iterator<String[]> it = tempOutputArray.iterator();
                while(it.hasNext()) appendFile(it.next());
                tempOutputArray.clear();
            }catch(Exception nullException){
            //Do nothing
            }
        }
    }

Upvotes: 1

Views: 460

Answers (2)

Gray
Gray

Reputation: 116878

Your problem is that you are modifying the future static field without synchronization in your timer-task thread(s) and reading it in the main thread. You need to either synchronize on it when you modify and read it or use another mechanism to share information between the threads.

I'd recommend switching from a static field to a LinkedBlockingQueue as a better way to send information from the PingNode call to the appendFile(...) method. This saves from needing to do the synchronization yourself and protects against the race conditions where multiple timer-tasks will start and overwrite the future before the consumer can get() from them. Maybe something like:

 BlockingQueue<String[]> queue = new LinkedBlockingQueue<String[]>();
 ...

 // inside of run, producer passes the queue into the PingNode
 public void run() {
     pool.submit(new PingNode(queue));
 }

 // consumer
 while (true) {
     String[] array = queue.take();
     ...
 }

This doesn't take into effect how you are going to stop the threads when you are done. If the timer task is killed the entity could add to the queue a termination object to stop the main loop.

Upvotes: 3

torquestomp
torquestomp

Reputation: 3344

A Future object is not a bin, like an ArrayList, it merely points to a single computational result. Because you only have one static pointer to this Future, what I imagine is happening is this:

    future = null
    nullException
    nullException
    nullException
    nullException
    ...
    First thread finally sets future = Future<ArrayList<String>>
    Call to future.get() blocks...
        Meanwhile, all other threads get scheduled, and they reassign future
        The last thread will obviously get the last say in what future points to
    Data is gathered, written to file, loop continues
    future now points to the Future from the last thread
    Results from last thread get printed

Upvotes: 0

Related Questions