chillworld
chillworld

Reputation: 4279

List getting threadsafe with removing items

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

Answers (1)

Jean Logeart
Jean Logeart

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

Related Questions