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<>();
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