Volatile access to array element

Question

I am having doubts about the volatile keyword in Java when it comes to arrays (after reading a few questions here and specially this document ).

I have a series of threads reading information from an int array named cardArray, and if an entry does not exist in the array I want to create it. What I use for that is a synchronized method like:

    public synchronized void growArray(int id) {
      if(getIndex(id) == -1) {
        cardArray = Arrays.copyOf(cardArray, cardArray.length + 1);
        cardArray[cardArray.length - 1] = id;
      }
    }

getIndex(int id) is a method which returns the position of such unique id in the array or -1 if the array does not exist. cardArray was declared like: protected volatile int[] cardArray = new int[10];

The idea behind this is that two threads may want to make the array grow at the same time. Since the growArray() method is synchronized, they will have to make it one by one. Once the first thread makes the array grow, the subsequent threads should not do it, since getIndex(id) should indicate that the id already exists. However, the fact that is the array what is declared as a volatile and not its elements makes me suspicious about if this will be the actual behavior.

Is this what will happen or may the following threads accessing growArray() still try to make the array grow? If so, is there any alternative apart from atomic arrays? I was thinking (in case this does not work as I want) to add cardArray = cardArray after writing the element, but I do not know if it would make a difference.

Why an array? There's other data structures more suitable to resizing. Also, what's `getIndex(int)` do? — blgt, Jun 13 '14 at 12:38
I am no expert in Java data structures, what other structure could I use for effective resizing? `getIndex(int id)` searches the array to see if id is already contained as an entry (for loop, array is not sorted). If it is, it returns its index. If it is not, -1. — user2891462, Jun 13 '14 at 12:40
I'd use a `HashSet` or one of the queues, depending on requirements. Is `getIndex()` synchronized? [Do you call it anywhere else?](http://stackoverflow.com/q/9525882/) Do any other (non-synchronized?) functions interact with the array? If the answers are all *no*, you needn't worry (and might as well make the array synchronized). Otherwise, you probably need some additional checking. — blgt, Jun 13 '14 at 12:53

score 1 · Accepted Answer · answered Jun 13 '14 at 13:30

Depending on your requirements, you may want to use ArrayList or HashSet or LinkedHashSet. If you're going to be writing much code in Java you'll really benefit from familiarizing yourself with the Java collections library.

Here is an explanation of some of the options available:

http://docs.oracle.com/javase/tutorial/collections/implementations/index.html

If you decide to use an ArrayList, your implementation would look like this:

// This is how you might create your list.
private List<Integer> myList = new ArrayList<Integer>();

public synchronized void growArray(int id) {
  if( ! cardList.contains(id) ){
    cardList.add(id);
  }
}

If you use a HashSet (which guarantees uniqueness and grows automatically) you might do this:

// This is how you might create your list.
private Set<Integer> myList = new HashSet<Integer>();

public synchronized void growArray(int id) {
  cardList.add(id);
}

Hope this helps!

Thank you! I think I am going to use a ConcurrentHashMap (since I am using that array as an index for another array of associated objects). ConcurrentHashMap includes atomic addition to the table with `putIfAbsent` and apparently eliminates the need of external synchronization in my case. — user2891462, Jun 13 '14 at 17:54

akostadinov · Answer 2 · 2014-06-13T12:52:59.390

0

I don't see any issue with that. As far as I understand the cardArray variable is volatile. You are changing the reference and since it is volatile the second thread should see the new value and work with the updated array.

Problem I see is if both of your threads try to insert same id at the same time so you end up with a duplicate value if that matters. So if one thread see a value missing, it will start to create a new array to insert it. But at the same time if another array is checking the same value, it will wait for the first thread to do its job. And then insert value without checking it already exists. You can fix that by checking for the value again.

Another issue is that if you have concurrent threads reading the array, there would be a short time when array will be longer but last value would not be what you expect. You may try to create array, update last value and lastly change the variable referencing it.

As comments say though, you'll be better using another data structure.

edited Jun 13 '14 at 12:52

answered Jun 13 '14 at 12:42

akostadinov

17,364
6
77
85

That's why the method is synchronized: only one thread can make use of it at a time, so whichever goes second would get into the method but should not do anything. Also, did you read the document I link in the question? – user2891462 Jun 13 '14 at 12:44
@user2891462, you are right. It should be fine. I didn't see your `getIndex(id)` method call. – akostadinov Jun 13 '14 at 12:47
Any suggestions for different data structures? – user2891462 Jun 13 '14 at 12:49
@user2891462, not sure what you're doing but see http://stackoverflow.com/questions/6720396/different-types-of-thread-safe-sets-in-java and http://stackoverflow.com/questions/6992608/why-there-is-no-concurrenthashset-against-concurrenthashmap – akostadinov Jun 13 '14 at 12:54

score 0 · Answer 3 · answered Jun 13 '14 at 13:26

Is this what will happen or may the following threads accessing growArray() still try to make the array grow?

If a thread exists the method with getIndex(id) == -1 being false, any thread that then enters the method will too see this update. This is because the method is synchronized. In fact you can remove volatile entirely and it would still work.

However, since you are updating the volatile, any non-synchronized access would benefit from the volatile write, so you may want to keep it declared that way.

Volatile access to array element

3 Answers3