Reputation: 4279
I'm trying to remove items from a list until its empty with multithreading.
Code:
public void testUsers() {
final List<User> users = userDao.findAll();
final int availableProcessors = Runtime.getRuntime().availableProcessors() * multiplier;
final List<String> loggingList = Lists.newArrayList();
final List<Integer> sizeChecked = Lists.newArrayList();
int totalSizeChecked = 0;
int sizeList = users.size();
ExecutorService executorService = Executors.newFixedThreadPool(availableProcessors);
for (int i = 0; i < availableProcessors; i++) {
createThread(executorService, users, loggingList, sizeChecked);
}
executorService.shutdown();
try {
// wait for all threads to die
executorService.awaitTermination(1, TimeUnit.HOURS);
} catch (InterruptedException ex) {
}
for (Integer count : sizeChecked) {
totalSizeChecked += count;
}
Assert.assertTrue(totalSizeChecked==sizeList);
}
private void createThread(ExecutorService executorService, final List<User> users,
final Collection<String> loggingList, final List<Integer> sizeChecked) {
executorService.execute(new Runnable() {
@Override
public void run() {
int totalChecked = 0;
while (!users.isEmpty()) {
User user = null;
synchronized (users) {
if (!users.isEmpty()) {
user = users.remove(0);
}
}
totalChecked++;
if (user != null) {
String reason = checkUser(user);
if (reason != null) {
loggingList.add(reason);
}
} else {
LOGGER.info("user is null");
}
}
sizeChecked.add(totalChecked);
}
});
}
Now I was thinking this couldn't be so wrong cause I made the list synchronised for removing the first item. I'm testing with a multiplier of 6.(on prod it will be lowered to 1-2) I get this in the email : The batch was not correctly executed. Size of accounts that must be checked : 28499. Size of accounts that have been checked: 25869
What do I wrong to get it threadsafe?
Upvotes: 0
Views: 40
Reputation: 53839
List<Integer> sizeChecked
is not thread safe. Therefore you cannot add elements in parallel in it.
Synchronize your add
operation or use a thread-safe structure. If sizeChecked
is just a counter, use an AtomicLong
instead and make each thread increment it.
Upvotes: 1