Java issues with notify() / wait()

Question

I have a generator inside an Iterator instance, which generates and returns one item every time next() is called on the iterator. It does seem to kind of work, but I am getting null values returned.

Class code:

package spiral;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
 *
 * @author student
 */
public class Spiral7DGenerator implements Iterator<List<Integer>> {
    private boolean releaseNext = false;
    private List<Integer> releaseList;

    public Spiral7DGenerator() {
        new Thread(new Runnable() {
            @Override
            public void run() {
                for (int t = 0; true; t++) {
                    loops(t);
                }
            }
        }).start();
    }

    @Override
    public boolean hasNext() {
        return true;
    }

    @Override
    public List<Integer> next() {
        synchronized(this) {
            releaseNext = true;
            notify();
            return releaseList;
        }
    }

    @Override
    public void remove() {
        throw new UnsupportedOperationException("Not supported yet.");
    }

    private void loops(int t) {
        for (int d1 = 0; d1 <= t; d1++) {
            for (int d2 = 0; d2 <= (t - d1); d2++) {
                for (int d3 = 0; d3 <= (t - d1 - d2); d3++) {
                    for (int d4 = 0; d4 <= (t - d1 - d2 - d3); d4++) {
                        for (int d5 = 0; d5 <= (t - d1 - d2 - d3 - d4); d5++) {
                            for (int d6 = 0; d6 <= (t - d1 - d2 - d3 - d4 - d5); d6++) {
                                int d7 = (t - d1 - d2 - d3 - d4 - d5 - d6);
                                generate(0, d1, d2, d3, d4, d5, d6, d7);
                            }
                        }
                    }
                }
            }
        }
    }

    private void generate(int pos, Integer... array) {
        if (pos == array.length) {
            List<Integer> list = new ArrayList<>();
            list.addAll(Arrays.asList(array));
            synchronized(this) {
                while (!releaseNext) {
                    try {
                        wait();
                    } catch (InterruptedException ex) {
                        Logger.getLogger(Spiral7DGenerator.class.getName()).log(Level.SEVERE, null, ex);
                    }
                }
            }
            releaseNext = false;
            releaseList = list;
            return;
        }
        generate(pos + 1, array);
        array[pos] = -array[pos];
        if (array[pos] != 0) {
            generate(pos + 1, array);
        }
    }
}

Testing code:

package spiral;

import org.junit.Test;
import static org.junit.Assert.*;

/**
 *
 * @author Beheerder
 */
public class Spiral7DGeneratorTest {

    public Spiral7DGeneratorTest() {
    }

    @Test
    public void testHasNext() {
    }

    @Test
    public void testNext() {
        System.out.println("test");
        Spiral7DGenerator s7dg = new Spiral7DGenerator();
        System.out.println("s7dg.next() = " + s7dg.next());
        System.out.println("s7dg.next() = " + s7dg.next());
        System.out.println("s7dg.next() = " + s7dg.next());
        System.out.println("s7dg.next() = " + s7dg.next());
    }

    @Test
    public void testRemove() {
    }

}

Test output:

test
s7dg.next() = null
s7dg.next() = null
s7dg.next() = null
s7dg.next() = null

I'm afraid it is just a simple thing, but I am overlooking it completely.

Answer 1

The idea here is that the worker thread feeds a stream of results to the main thread, correct?

I suggest that you do not attempt to construct the concurrency control logic by hand, but instead use a SynchronousQueue. The code would look like:

public class Spiral7DGenerator implements Iterator<List<Integer>> {
    private BlockingQueue<List<Integer>> spirals = new SynchronousQueue<List<Integer>>();

    @Override
    public List<Integer> next() {
        return spirals.take();
    }

    private void generate(int pos, Integer... array) {
        if (pos == array.length) {
            List<Integer> list = new ArrayList<>();
            list.addAll(Arrays.asList(array));
            spirals.put(list);
            return;
        }
    // etc
    }
}

Answer 2

I haven't fully understood your code but two points I can make:

• releaseList must be declared volatile (at least) as it is being assigned outside a synchronized block.
• your next method does not wait for the notified thread to wake up and produce a result. This is a race condition. You need next to wait on an extra mutex (or something) to allow the generator to notify that releaseList has been set. (Actually, when you do this, you won't need to make releaseList volatile any more.)