Java concurrency problem - Locks and Synchronize method

I am working on Re entrant locks and trying to map it to Synchronize. However, both of these classes are giving me unexpected results. I expect arrayList to have a value between 0 and 9. but that value never comes in both of these programs. Please suggest. With blocking:

package Threads;

import java.util.ArrayList;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

public class Locking {

    Lock lock = new ReentrantLock(true);
    ArrayList<Integer> al = new ArrayList<Integer>();
    static int count = 0;

    public void producer() {
        lock.lock();
        count++;
        al.add(count);
        System.out.println(al);
        try {
            Thread.sleep(5000);
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        } finally {
            lock.unlock();
        }
        // System.out.println("I came out of this block:"+Thread.currentThread().getName());
    }

    public void consumer() {
    }

    public static void main(String[] args) {
        // ExecutorService ex= Executors.newCachedThreadPool();
        ExecutorService ex = Executors.newFixedThreadPool(10);

        for (int i = 0; i < 10; i++) {
            ex.submit(new Runnable() {

                @Override
                public void run() {
                    try {
                        Thread.sleep(2000);
                    } catch (InterruptedException e) {
                        // TODO Auto-generated catch block
                        e.printStackTrace();
                    }
                    new Locking().producer();
                }

            });
        }
        ex.shutdown();
    }
}

      

Synchronized:

package Threads;

import java.util.ArrayList;
import java.util.Collections;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

public class LockwithSynchronize {

    ArrayList<Integer> al = new ArrayList<Integer>();
    static int count = 0;

    public synchronized void producer() {
        count++;
        al.add(count);
        System.out.println(al);
        try {
            Thread.sleep(5000);
            // System.out.println("I came out of this block:"+Thread.currentThread().getName());
        } catch (InterruptedException e) {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }

    public static void main(String[] args) {
        // ExecutorService ex= Executors.newCachedThreadPool();
        ExecutorService ex = Executors.newFixedThreadPool(10);

        for (int i = 0; i < 10; i++) {
            ex.submit(new Runnable() {
                @Override
                public void run() {
                    try {
                        Thread.sleep(2000);
                    } catch (InterruptedException e) {
                        // TODO Auto-generated catch block
                        e.printStackTrace();
                    }
                    new LockwithSynchronize().producer();
                }
            });
        }
        ex.shutdown();
    }

}

      

+3


source to share


2 answers


There are so many mistakes in this.

First, you expect it to contain 0..9, but you call count++

before al.add(count)

, which means you should really expect 1..10.

Since you use a new one LockWithSynchronize

or Locking

every time, there is really no shared lock - each instance gets its own lock, which never conflicts with any other lock, which means your count

variable is completely unprotected. And you probably get intermittently either corrupted ArrayList

due to add

being called on multiple threads without synchronization, ConcurrentModificationException

or similar.

Try the following:



public static void main(String [] args){

    ExecutorService ex= Executors.newFixedThreadPool(10);

    final Locking producer = new Locking();

    for (int i=0; i<10;i++)
    {
        ex.submit(new Runnable(){

            @Override
            public void run() {
                try {
                    Thread.sleep(2000);
                } catch (InterruptedException e) {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }
                producer.producer();
            }

        });
    }

    ex.shutdown();
}

      

Note that instead of a new Locking

one, we reuse the same instance every time, and now you expect what you expect: you are very quickly generating ten threads, each waiting two seconds before trying to call producer()

. One thread acquires the lock and the other nine blocks. The one that acquires the lock then waits with a lock for five seconds before exiting, after which the next thread acquires the lock, waits for five seconds and exits, etc. Thus, it will take about a minute to run.

A similar modification fixes another.

+3


source


Your sync doesn't protect a static variable. Synchronization only locks the monitor of the current object this

while it is being used. The noproduct () call will never wait for anything else in the code you posted. Lock something in common - say LockWithProducer.class.



0


source







All Articles