synchronized Threads - Homework

Question

I need to run a java program called ArrayHolder that will run two Threads. ArrayHolder will have an Array. ThreadSeven will overwrite every element of that Array with 7, and ThreadOne with 1. The result after execution should be 7,1,7,1,7,1,7,1 etc. I have solved this problem, although I dont like my solution and was hoping you could suggest a better way.

p.s: Both Threads must write on all indexes.

public class ArrayHolder {

    private int[] array = {1, 2, 3, 4, 5, 6, 4, 8, 9, 10};

    public void writeInt(int pos, int num) {
        array[pos] = num;
    }

    public static void main(String[] args) {
        ArrayHolder holder = new ArrayHolder();
        ThreadSeven seven = new ThreadSeven(holder, null);
        Runnable one = new ThreadOne(holder, seven);
        Thread thread1 = new Thread(seven);
        Thread thread2 = new Thread(one);
        seven.setThread(one);

        thread1.start();
        thread2.start();

        holder.printArray();
    }

    private void printArray() {
        for (int i = 0; i < 10; i++) {
            System.out.println(array[i]);
        }
    }

public class ThreadSeven implements Runnable {
    private ArrayHolder array;
    private Runnable t;
    private int flag=0;
    @Override
    public void run() {
        for(int i=0;i<10;i++){
            array.writeInt(i, 7);

            flag=(flag+1)%2;
            if (flag==0){
                synchronized(t){
                    t.notify();
                }
            }else{
                synchronized(this){
                    try {
                        this.wait();
                    } catch (InterruptedException ex) {
                        Logger.getLogger(ThreadSeven.class.getName()).log(Level.SEVERE, null, ex);
                    }
                }
            }
        }
    }
    public ThreadSeven (ArrayHolder ar,Runnable t){
        array=ar;
        this.t=t;
    }
    public void setThread(Runnable t){
        this.t=t;
    }
}

public class ThreadOne implements Runnable {

    private ArrayHolder array;
    private Runnable t;
    private int flag = 0;

    @Override
    public void run() {
        for (int i = 0; i < 10; i++) {
            array.writeInt(i, 1);

            flag = (flag + 1) % 2;
            if (flag == 1) {
                synchronized (t) {
                    t.notify();
                }
            } else {
                synchronized (this) {
                    try {
                        this.wait();
                    } catch (InterruptedException ex) {
                        Logger.getLogger(ThreadSeven.class.getName()).log(Level.SEVERE, null, ex);
                    }
                }
            }
        }
    }

    public ThreadOne(ArrayHolder ar, Runnable t) {
        array = ar;
        this.t = t;
    }

    public void setThread(Runnable t) {
        this.t = t;
    }
}

Answer 1

Your solution has some problems and looks to me like it would not print the correct result.

a) You don't wait for the threads to finish before you print the resulting array

Add thread1.join() and thread2.join() before holder.printArray() in case it is not there yet.

b) both threads start with writing immediately via array.writeInt(0, /* 1 or 7 */); After that they start to wait on each other. Whether the first index is correct or not depends on luck.

c) Continuing after this.wait(); without a loop checking a condition is not safe since the interrupt could be caused by something else than the other thread. I guess it's okay to do that here since it's just an exercise.

d) I see a potential deadlock: let's assume both threads are still writing the first index. So both are not in a synchronized block.

The thread that has to notify the other one does so, writes the next index and goes into it's own wait block. But the second thread was not waiting at the time so the notify from the first thread did nothing. Second thread goes into wait block too.

Now both threads wait on each other and nothing happens anymore.

I don't have a great simple solution for you since that problem is quite complex.

The 1-thread needs to start writing at index 0 then wait until the 7-thread has written index 0 and 1, now 1-thread writes index 1 and 2 and waits and so on. That is the only way I see possible to ensure that both thread have written to every index and that the result is 7-1-7-1-.... Synchronizing access inside ArrayHolder would be very tricky since it needs to make sure that both threads have written to each index in the correct order.

But I think your general idea is okay. You just need to make sure that it is safe

Answer 2

ThreadSeven and ThreadOne don't need to be separate classes; is looks like you just copy/pasted the code, and then changed the 7 in writeInt to a 1. Instead, you can paramaterize this value and pass it in the constructor. Then you get something like:

public class ThreadWriter implements Runnable {
    private final int numberToWrite;
    // ...
    public ThreadOne(ArrayHolder ar, Runnable t, int numberToWrite) {
        array = ar;
        this.t = t;
        this.numberToWrite = numberToWrite;
    }
    // ...
}

Another point is that both of your threads have to know about each other; this doesn't scale well. Pretend that for your next assignment your teacher said that you have to handle three threads which write 1, 4, 7, 1, 4, 7, ...; you would have to change the implementation of ThreadOne and ThreadSeven. A better solution which you could make now is have the ThreadWriters themselves be dumber, and manage their interaction more in the ArrayHolder class (or with an intermediary ThreadWriterManager class).