The wonders of Java ConcurrentLinkedQueue

So, I'm trying to use a threadpool along with a ConcurrentLinkedQueue to make a bunch of downloads. My problem is that in the below (runnable) code, the set from and to Calendar dates of the individual SingleDownloads get switched around randomly when the Threads start. The reason I'm puzzled by this is because ConcurrentLinkedQueue is thread-safe. I must be making a mistake somewhere.

SAMPLE OUTPUT:
app30 2014-12-24 2015-01-23
app29 2014-12-26 2015-01-24
app28 2014-12-28 2015-01-25
...
OUTPUT OMITTED
...
Downloading: app29 2014-12-26 2015-01-24
Downloading: app28 2014-12-30 2015-01-26
Downloading: app30 2014-12-26 2015-01-24

As you can see above, the dates get changed around randomly.

import java.io.File;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.concurrent.ConcurrentLinkedQueue;

public class DownStack {

    private static SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd");
    private volatile static boolean running = true;
    private volatile static int threadsCompleted;
    private static ConcurrentLinkedQueue<Runnable> taskQueue;
    private static DownloadThread[] downloadThreads;
    private static String[] status;

    static class SingleDownload {
        String app;
        Calendar from;
        Calendar to;
        File folder;

        public SingleDownload(String app, Calendar from, Calendar to,
                File folder) {
            this.app = app;
            this.from = from;
            this.to = to;
            this.folder = folder;
        }
    }

    static class DownloadTask implements Runnable {
        private SingleDownload param;
        private int index;

        public DownloadTask(SingleDownload param, int index) {
            this.param = param;
            this.index = index;
        }

        public void run() {
            status[index] = "downloading";
            downloadOne(param.app, param.from, param.to, param.folder);
            status[index] = "finished";
        }
    }

    private static class DownloadThread extends Thread {
        public void run() {
            try {
                while (running) {
                    Runnable task = taskQueue.poll();
                    if (task == null)
                        break;
                    task.run();

                }
            } catch (Exception e) {
                e.printStackTrace();
            } finally {
                threadFinished();
            }
        }
    }

    synchronized private static void threadFinished() {
        threadsCompleted++;
        if (threadsCompleted == downloadThreads.length) {
            running = false;
            downloadThreads = null;
        }
    }

    public static void downloadOne(String app, Calendar from, Calendar to,
            File folder) {
        System.out.println("Downloading: " + app + " "
                + sdf.format(from.getTime()) + " " + sdf.format(to.getTime()));
    }

    public static void main(String[] args) {
        File folder = new File(".");
        ArrayList<SingleDownload> downloadList = new ArrayList<>();
        for (int i = 30; i > 0; i--) {
            Calendar cal1 = Calendar.getInstance();
            cal1.add(Calendar.DAY_OF_YEAR, -2 * i);
            Calendar cal2 = Calendar.getInstance();
            cal2.add(Calendar.DAY_OF_YEAR, -1 * i);
            downloadList.add(new SingleDownload("app" + i, cal1, cal2, folder));
        }

        for (SingleDownload sd : downloadList) {
            System.out.println(sd.app + " " + sdf.format(sd.from.getTime())
                    + " " + sdf.format(sd.to.getTime()));
        }

        status = new String[downloadList.size()];
        int index = 0;
        taskQueue = new ConcurrentLinkedQueue<Runnable>();
        for (SingleDownload fd : downloadList) {
            Runnable r = new DownloadTask(fd, index);
            taskQueue.add(r);
            index++;
        }
        int threadCount = 20;
        downloadThreads = new DownloadThread[threadCount];
        running = true;
        threadsCompleted = 0;
        for (int i = 0; i < threadCount; i++) {
            downloadThreads[i] = new DownloadThread();
            try {
                downloadThreads[i].setPriority(Thread.currentThread()
                        .getPriority() - 1);
            } catch (Exception e) {
            }
            downloadThreads[i].start();
        }

        while (running) {
            try {
                Thread.sleep(1000);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            // downloadTable.repaint();
        }

    }
}
Jon Skeet
people
quotationmark

I believe the problem is due to your use of SimpleDateFormat - it isn't thread-safe. If you create a new SimpleDateFormat each time you need to use it, your results should be stable. (The order won't be, but that's okay.)

If you possibly can, I'd strongly recommend that you use either Joda Time or the java.time package of Java 8, instead of Date/Calendar/SimpleDateFormat - they're much better APIs, and the equivalents of SimpleDateFormat are thread-safe.

people

See more on this question at Stackoverflow