Thread safety when repeating through ArrayList using foreach

I have an ArrayList that is created and populated by a background thread (I use it to store Cursor data). At the same time, it can be accessed by the main thread and iterated using foreach. So this can obviously lead to an exception.

My question is what is the best way to make this class field thread safe without copying it every time or using flags?

 class SomeClass { private final Context mContext; private List<String> mList = null; SomeClass(Context context) { mContext = context; } public void populateList() { new Thread(new Runnable() { @Override public void run() { mList = new ArrayList<>(); Cursor cursor = mContext.getContentResolver().query( DataProvider.CONTENT_URI, null, null, null, null); try { while (cursor.moveToNext()) { mList.add(cursor.getString(cursor.getColumnIndex(DataProvider.NAME))); } } catch (Exception e) { Log.e("Error", e.getMessage(), e); } finally { if (cursor != null) { cursor.close(); } } } }).start(); } public boolean searchList(String query) { // Invoked on the main thread if (mList != null) { for (String name : mList) { if (name.equals(query) { return true; } } } return false; } } 
+6
source share
4 answers

In general, it is a very bad idea to work simultaneously with a data structure that is not thread safe. You have no guarantee that the implementation will not change in the future, which can greatly affect the behavior of the application runtime, that is, java.util.HashMap causes infinite loops while changing at the same time.

For simultaneous access to the list, Java provides java.util.concurrent.CopyOnWriteArrayList . Using this implementation will solve your problem in various ways:

  • it is thread safe allowing simultaneous modifications
  • Iterating over snapshots of a list is independent of simultaneous add operations, allowing you to add and iterate at the same time
  • faster than synchronization

Alternatively, if you do not use a copy of the internal array, this is a strict requirement (which I cannot imagine in your case, the array is quite small, since it contains only references to objects that can be copied to memory quite efficiently), you can synchronize access on the map . But this requires that the card be correctly initialized, otherwise your code may throw a NullPointerException , because the flow order is not guaranteed (you assume that populateList() launched earlier, so the list is initialized. When using a synchronized block, choose a protected block wisely. If you have all the contents of the run() method in a synchronized block, the reader thread should wait for the results to be processed from the cursor, which may take some time, so you really lose all concurrency.

If you decide to switch to a synchronized block, I would make the following changes (and I do not claim that they are absolutely correct):

Initialize the list box so that we can synchronize access with it:

 private List<String> mList = new ArrayList<>(); //initialize the field 

Synchronize the modification operation (add). Do not read the data from the cursor inside the synchronization block, because if its operation has a low delay, during this operation it was not possible to read the millimeter, which blocks all other flows for a rather long time.

 //mList = new ArrayList<>(); remove that line in your code String data = cursor.getString(cursor.getColumnIndex(DataProvider.NAME)); //do this before synchronized block! synchronized(mList){ mList.add(data); } 

The reading iteration should be inside the synchronization block, so not a single element is added, but repeated:

 synchronized(mList){ for (String name : mList) { if (name.equals(query) { return true; } } } 

So, when two threads work in a list, one thread can either add one item or iterate over the entire list at a time. You do not have parallel execution in these pieces of code.

Regarding synchronized versions of the list (i.e. Vector , Collections.synchronizedList() ). They can be less efficient, because during synchronization you actually lose parallel execution, since only one thread can start protected blocks at a time. In addition, they may still be subject to ConcurrentModificationException , which can occur even on a single thread. It throws if the data structure changes between creating an iterator and an iterator. Therefore, these data structures do not solve your problem.

I also do not recommend manual synchronization, because the risk of simply making a mistake is too high (synchronization on the wrong or another monitor, synchronization blocks too large, ...)

TL DR

use java.util.concurrent.CopyOnWriteArrayList

+3
source

You can use Vector , which is equivalent to an ArrayList stream.

EDIT: Thanks to the comment on the file , now I know that this does not prevent ConcurrentModificationException from using multiple threads:

Only individual calls are synchronized. Thus, one add cannot be called, while another thread causes the add, for example. But changing the list will cause CME to be thrown when it repeats in another thread. You can read the iterator docs on this topic.

Also interesting:

In short: DO NOT use Vector .

+1
source

Use Collections.synchronizedList(new ArrayList<T>());

Example:

 Collections.synchronizedList(mList); 
+1
source

java synchronized block http://www.tutorialspoint.com/java/java_thread_synchronization.htm

 class SomeClass { private final Context mContext; private List<String> mList = null; SomeClass(Context context) { mContext = context; } public void populateList() { new Thread(new Runnable() { @Override public void run() { synchronized(SomeClass.this){ mList = new ArrayList<>(); Cursor cursor = mContext.getContentResolver().query( DataProvider.CONTENT_URI, null, null, null, null); try { while (cursor.moveToNext()) { mList.add(cursor.getString(cursor.getColumnIndex(DataProvider.NAME))); } } catch (Exception e) { Log.e("Error", e.getMessage(), e); } finally { if (cursor != null) { cursor.close(); } } } } }).start(); } public boolean searchList(String query) { // Invoked on the main thread synchronized(SomeClass.this){ if (mList != null) { for (String name : mList) { if (name.equals(query) { return true; } } } return false; } } } 
+1
source

All Articles