Java Threading- Am I writing thread-safe code?

Question

I'm currently trying to get into parallel processing, so to do this, I'm writing a program that processes an image, giving information about its color values overall- I am doing some tests on this one class with a randomly generated array of integers, and 4 threads are running to process every 4th pixel, from their respective starting places. I was just wondering if this read is thread-safe? Can multiple threads read the same data structure if that's what I want?

import java.awt.image.BufferedImage;
import java.lang.Thread;


public class ImageProcessor extends Thread {

    public static void main(String[] args) {

        int[] z = new int[10000000];
        for (int i = 0; i < 10000000; i++) {

            double a = (Math.random()*1000000);
            z[i] = (int) a;

        }

        ImageProcessor ip = new ImageProcessor();
        ip.imgRGBPercent(z);
    }


    public ImageProcessor() {

    }

    public void process(int[] x, int startPoint) {

        (new Thread(new ReadThread(x, startPoint))).start();    
    }

    public int[] imgRGBPercent(int[] x) {




        ReadThread first = new ReadThread(x, 0);
        ReadThread second = new ReadThread(x, 1);
        ReadThread third = new ReadThread(x, 2);
        ReadThread fourth = new ReadThread(x, 3);

        Thread a = (new Thread(first));
        Thread b = (new Thread(second));
        Thread c = (new Thread(third));
        Thread d = (new Thread(fourth));

        long timeMetric = System.currentTimeMillis();
        a.start();
        b.start();
        c.start();
        d.start();


        try {

            a.join();
        }
        catch (Exception e) {

        }

        try {

            b.join();
        }
        catch (Exception e) {

        }

        try {

            c.join();
        }
        catch (Exception e) {

        }

        try {

            d.join();
        }
        catch (Exception e) {

        }

        int redTotal, blueTotal, greenTotal;

        redTotal = first.getRGBTotals()[0] + second.getRGBTotals()[0] + third.getRGBTotals()[0] + fourth.getRGBTotals()[0];
        blueTotal = first.getRGBTotals()[1] + second.getRGBTotals()[1] + third.getRGBTotals()[1] + fourth.getRGBTotals()[1];
        greenTotal = first.getRGBTotals()[2] + second.getRGBTotals()[2] + third.getRGBTotals()[2] + fourth.getRGBTotals()[2];

        System.out.println(greenTotal);

        System.out.println(System.currentTimeMillis() - timeMetric);

        timeMetric = System.currentTimeMillis();

        ColorValue cv1 = new ColorValue();
        int sum = 0;
        int sum1 = 0;
        int sum2 = 0;
        for (int i = 0; i < x.length; i++) {

            sum += cv1.getGreen(x[i]);
            sum1 += cv1.getRed(x[i]);
            sum2 += cv1.getBlue(x[i]);
        }


        System.out.println(sum);

        System.out.println(System.currentTimeMillis() - timeMetric);

        int[] out = new int[3];
        return out;
    }


    private class ReadThread implements Runnable {

        private int[] colorArr;
        private int startPoint, redTotal, blueTotal, greenTotal;
        private ColorValue cv;

        public ReadThread(int[] x, int startPoint) {

            colorArr = x;
            this.startPoint = startPoint;
            cv = new ColorValue();
        }

        @Override
        public void run() {

            //System.out.println("hit");

            for (int i = startPoint; i < colorArr.length; i+=4 ) {
                redTotal += ColorValue.getRed(colorArr[i]);
                blueTotal += ColorValue.getBlue(colorArr[i]);
                greenTotal += ColorValue.getGreen(colorArr[i]);

            }   

        }

        public int[] getRGBTotals() {

            int[] out = new int[3];
            out[0] = redTotal;
            out[1] = blueTotal;
            out[2] = greenTotal;

            return out;
        }
    }

}

Answer 1

This logic would concern me a little:

 for (int i = startPoint; i < colorArr.length; i+=4 ) {
     redTotal += ColorValue.getRed(colorArr[i]);
     blueTotal += ColorValue.getBlue(colorArr[i]);
     greenTotal += ColorValue.getGreen(colorArr[i]);
 }   

colorArr is a reference to an array; the reference was passed to the Runnable during the constructor, but the array itself was created outside.

In the complete program you posted, I don't think it's a problem, since this array isn't modified anywhere in your program after the point where you start the threads. But in a larger, "real-world" case, you have to be aware that you're reading colorArr[i] three times and the value may not be the same each time, if there are other threads that could make changes to colorArr. That's one of the things you have to watch out for when writing concurrent code. This would be a little better:

 for (int i = startPoint; i < colorArr.length; i+=4 ) {
     int color = colorArr[i];
     redTotal += ColorValue.getRed(color);
     blueTotal += ColorValue.getBlue(color);
     greenTotal += ColorValue.getGreen(color);
 }   

But depending on what your needs are, you may need to take extra steps to make sure no part of the program can modify colorArr at any point while the entire loop is running. Then you need to start looking into lock objects and synchronized, and you'd want to seriously consider setting up a separate class for the colorArr, with methods for modifying and reading the array that are either synchronized methods or contain logic to ensure that things are synchronized properly--by putting the array in its own class, the needed synchronization logic could be encapsulated in that class, so clients of the class wouldn't have to worry about it. That's the kind of thing you need to think about when you start using concurrency.

Answer 2

Yes, multiple threads can read the same objects and are fine so long as other threads aren't modifying them at the same time. Depending on what you're doing the Fork-Join framework may be useful, it manages a lot of the threading details for you, so would be worth investigating.

Answer 3

Yes. As long as the data structure is not modified while it's being read, you're safe. Every write done before starting a thread will be visible by the started thread.