0

As volatile only applies to variable reference, is there anyway to make write(add) to List from one thread to be visible to another thread?

My code has two threads, one thread A pushes a lot of items to a List, another thread B reads the List after thread A finishes; two threads are synced by CountDownLatch. It happens several times when thread B read the List, not all items in List are visible.

My code looks like (the actual code is more complicated than this, the below code is too simple to reproduce the issue):

public class TestList {
    @Test
    public void test() throws InterruptedException {
        int num = 1000;
        final CountDownLatch latch = new CountDownLatch(1000);
        final List<Integer> list = new ArrayList<Integer>();

        for (int i=0; i<num; i++) {
            final int finalI = i;
            Thread t = new Thread(new Runnable() {
                @Override
                public void run() {
                    list.add(finalI);
                    latch.countDown();
                }
            });
            t.start();
        }

        latch.await(1, TimeUnit.MINUTES);

        System.out.println(list.size());
    }
}

UPDATE: thanks for all answers. As @MikeStrobel's statement, I finally realize it's not only a memory visibility issue but also a synchronization issue. So the solution should be Collections#synchronizedList(...) as Kevin said, or synchronized keyword

zx_wing
  • 1,918
  • 3
  • 26
  • 39

3 Answers3

2

See Collections#synchronizedList(...)

Kevin Krumwiede
  • 9,868
  • 4
  • 34
  • 82
  • 3
    Beware though. Using a synchronized list doesn't magically solve all concurrency problems. For example, iterations and chec-then-act action must still be explicitely synchronized. – JB Nizet Aug 22 '14 at 19:24
1

Change:

t.run();

To:

t.start();

To start a thread actually. Calling run() method is just like a normal method call in the same thread.


Use 2 CountDownLatch to get it done in proper way.

sample code:

int num = 1000;
final CountDownLatch startSignal = new CountDownLatch(1);   // start from 1
final CountDownLatch doneSignal = new CountDownLatch(1000); // till 1000

final List<Integer> list = new ArrayList<Integer>();

for (int i = 0; i < num; i++) {
    final int finalI = i;
    Thread t = new Thread(new Runnable() {
        @Override
        public void run() {
            startSignal.await(); // wait for start signal
            list.add(finalI);
            doneSignal.countDown(); // count down the done signal
        }
    });
    t.start();
}
startSignal.countDown(); // let all threads proceed
doneSignal.await(1, TimeUnit.MINUTES); // wait for all to finish

System.out.println(list.size()); // prints 1000

Have a look at ExecutorService as well.

Braj
  • 46,415
  • 5
  • 60
  • 76
1

You're not checking the result of CountDownLatch.await(). So if it returns because of the timeout (i.e. if it returns false), the producer threads have not finished their job, and you can't access the list safely.

The CountDownLatch class offers visibility guarantees, if used properly.

JB Nizet
  • 678,734
  • 91
  • 1,224
  • 1,255
  • yeah, the code here is for illustration; the actual code is too complicated to put here. I want to know, in generic, how to make List visible among threads; there may be no CountDownLatch in place – zx_wing Aug 22 '14 at 19:12
  • That's a much too broad question. Read "Concurrency in Practice", by Brian Goetz. – JB Nizet Aug 22 '14 at 19:16
  • 2
    The issue is not just visibility; what happens when two threads call `add()` at the same time? Any number of things could happen with unsynchronized access, including both calls inserting items into the same slot, but increasing the count by two. Or one adding an item while another is in the process of resizing the internal array. This is not safe at all. – Mike Strobel Aug 22 '14 at 19:16
  • @MikeStrobel Agreed. I concentrated my answer on visibility issues and completely ignored the rest. You should post that as an answer. I'd upvote it. – JB Nizet Aug 22 '14 at 19:18
  • @zx_wing: I have to agree with other posters that your question is too broad. There's no way "in generic" to make List visible to other threads. **ALL** of the answers here are correct. You have to select the scheme that works best for your design. I second that you read Java Concurrency in Practice by Brian Goetz. – markspace Aug 22 '14 at 19:22