Shared list not updated when threads are queued

Question

I have a code which has two executors, executorShops and executorSections, the first one is not important though.

I create as many tasks as sections there are (three in this example), however, only two are executed at the same time.

Every task updates a shared list, but, the problem is only the first two threads update it correctly. The third thread, which has been queued, won't update it.

Here's the code where tasks are created:

Runnable task = () -> {
    LOG.info("Llamamos al ScraperManager para obtener el scraper de " + shop.getName());
    Scraper scraper = ScraperManager.getScraper(shop);
    LOG.info("Scraper de " + shop.getName() + " obtenido");

    ExecutorService executorSections = Executors.newFixedThreadPool(Properties.MAX_THREADS_SECTIONS);
    Set<Callable<List<Product>>> listOfTasks = new HashSet<>();    

    for (int j = 0; j < shop.getSections().size(); j++)
    {
        final Section section = shop.getSections().get(j);

        Callable<List<Product>> taskSection = () -> scraper.scrap(shop, section);

        listOfTasks.add(taskSection);                    
    }

    try 
    {
        List<Future<List<Product>>> listOfFutures = executorSections.invokeAll(listOfTasks);
        List<Product> productList = listOfFutures.get(shop.getSections().size() - 1).get();

        RestClient restClient = new RestClient(new URL(Properties.SERVER));

        restClient.saveProducts(productList, shop);

        countDownLatch.countDown();

        executorSections.shutdown();

    } catch (InterruptedException | ExecutionException ex ) {
        ...

    } catch (MalformedURLException ex) {
        ...
    }
};

And here's the scrap task:

public class ShopScraper implements Scraper
{
    private static List<Product> productList = Collections.synchronizedList(new ArrayList<>());

    private static final ThreadLocal<Boolean> threadFinished = 
            new ThreadLocal<Boolean>() 
            {
                @Override 
                protected Boolean initialValue() 
                {
                    return false;
                }
            };

    @Override
    public List<Product> scrap(Shop shop, Section section) throws IOException
    {
        // add products to the list

        return productList;
    }
}

EDIT: If I limit the number of threads to 1, then the second and third thread don't update the list.

Does anyone know what I did wrong?

Thanks in advance,

Would you mind trying to exchange CopyOnWriteArraylist for a [synchronizedList](https://docs.oracle.com/javase/8/docs/api/java/util/Collections.html#synchronizedList-java.util.List-) - wrapped ArrayList ? — Fildor, Oct 17 '16 at 07:58
http://stackoverflow.com/questions/17853112/in-what-situations-is-the-copyonwritearraylist-suitable — Antoniossss, Oct 17 '16 at 08:05
@Antoniossss Should I do a list.iterator() in the main code to read the latest value? Or just change it to another list implementation? — Dani M, Oct 17 '16 at 08:13
`CopyOnWriteArraylist` main purpose is to suppress `ConcurrentModificationException` that is thrown when one thread is iterating the list and another tries to modify that list. If you switch to synchronized implementation, everything should be fine. — Antoniossss, Oct 17 '16 at 08:16
@Antoniossss ok, will give it a try asap, although neither thread iterates over the list. They just modify it. — Dani M, Oct 17 '16 at 08:20
Yes, and that's where **Copy**OnWrite is a bad choice. Concurrent writes are expensive. And the more content, the more expensive. That doesn't explain the behavior observed, though. — Fildor, Oct 17 '16 at 08:22
I suspect this line: `if (hasEveryoneFinished(finishedSections))` Where do you set finishedSections and what is the implementation of hasEveryoneFinished? — Fildor, Oct 17 '16 at 08:25
OK, have tried: [ExecutorService.invokeAll](https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ExecutorService.html#invokeAll-java.util.Collection-) and just get() on each item of the returned list. The last get will return your complete List. You have to wait for all anyway and `get` on an earlier completed Future will return immediatly. Same effect, less complicated. — Fildor, Oct 17 '16 at 08:33
Even just counting the number of finished sections had been less complicated than that boolean construction ... — Fildor, Oct 17 '16 at 08:37
@Fildor, ok, I'll try it. About your second comment. The array is used for other purposes too. — Dani M, Oct 17 '16 at 08:39
Finally fixed it. Actually it was because a boolean local variable was accesed by every thread. Anyway, thanks for your comments, I've coded it as you've commented! — Dani M, Oct 17 '16 at 18:15

score 0 · Answer 1 · answered Oct 17 '16 at 07:28

0

1 thread = 1 task at the time.

    // MAX_THREADS_SECTIONS = 2
    ExecutorService executorSections = Executors.newFixedThreadPool(Properties.MAX_THREADS_SECTIONS);

You are limiting your pool size to only 2 threads, so only 2 tasks will be run in parallel.

Increase that and all the tasks will run in the same time.

answered Oct 17 '16 at 07:28

Antoniossss

31,590
6
57
99

Yeah, I know, but I need to limit the number of threads to be run in parallel. – Dani M Oct 17 '16 at 07:34
@cuoka Limit explains why 2 tasks are run insteed of 3. Third task however should be run after one of previous 2 would finish so conclusion is - submited tasks never ends. You have to show what `ShopScraper#scrap` does as most probably there is a deadlock over there. – Antoniossss Oct 17 '16 at 07:39
The three tasks are run, first two in parallel, the third one after one is finished, and every thread finishes, as the block inside the "if (hasEveryoneFinished(finishedSections))" is executed. My problem is that the third thread does not update the list, when the first two do. – Dani M Oct 17 '16 at 07:43
@http://stackoverflow.com/questions/17853112/in-what-situations-is-the-copyonwritearraylist-suitable – Antoniossss Oct 17 '16 at 08:05

Shared list not updated when threads are queued

1 Answers1