CODEWITHSUNDEEP
javaconcurrencyjava.util.concurrent
IUnknown
IUnknown

Reputation: 9829

Using boolean to synchronize

Is the following code thread-safe on concurrent access to List?
Does the volatile qualifies add any value here? 

   class concurrentList{

        private AtomicBoolean locked = new AtomicBoolean(true);
        volatile List<Integer> list=new LinkedList<Integer>();
        long start = System.currentTimeMillis();
        long end = start + 60*100;


        public void push(int e){
            while(!locked.get());
            list.add(e);
            while(!locked.compareAndSet(true,false));
        }

        public int pop(){
            int elem;
            while(locked.get());
            elem=(Integer)list.remove(0);
            while(!locked.compareAndSet(false,true));
            return elem;
        }
....
}

Upvotes: 3

Views: 728

Answers (4)

gati sahu
gati sahu

Reputation: 2641

Adding more to already added in answer i.e For any concurrent programming three concepts, you need to consider while writing a thread safe programming .When a concurrent program is not correctly written, the errors tend to fall into one of the three categories: Aomicity, Visibility, or Ordering.

Atomicity :deals with which actions and sets of actions have indivisible effects.It is usually thought of in terms of mutual exclusion.

Visibility :Determines when the effects of one thread can be seen by another.

Ordering :Determines when actions in one thread can be seen to occur out of order with respect to another

In your code First issue it is failing all mentioned above concepts.You are not using lock so the visibility and ordering will not be ensure. For thread safe you can use ReadWriteLock in concurrent API.Or there is available non-blocking linked-list which will use compareAndset.

Upvotes: 0

Beri
Beri

Reputation: 11620

Inn such cases I would recommend use ReadWriteLock. This lock has two uses. When readLock is on, no reads are allowed, untill write lock is released.Read lock is non blocking:

class concurrentList{
    ReadWriteLock lock =new ReentrantReadWriteLock();

    private AtomicBoolean locked = new AtomicBoolean(true);
    volatile List<Integer> list=new LinkedList<Integer>();
    long start = System.currentTimeMillis();
    long end = start + 60*100;


    public void push(int e){
        lock.writeLock().lock();
        try{
          list.add(e);
        } finally {
            lock.writeLock().unlock();
        }
    }

    public int pop(){
        lock.readLock().lock();
        try {
        int elem;

        elem=(Integer)list.remove(0);
        } finally {
           lock.readLock().unlock();
        }
        return elem;
    }

.... }

Upvotes: 1

developer_hatch
developer_hatch

Reputation: 16214

In this particulary case, I would use synchronized for the methods, and not volatile for the variable according to [this question][1]

class concurrentList{

    private AtomicBoolean locked = new AtomicBoolean(true);
    List<Integer> list=new LinkedList<Integer>();
    long start = System.currentTimeMillis();
    long end = start + 60*100;


    public synchronized void push(int e){
        while(someLock.notCondition()); //only an example
        list.add(e);
        someLock.notify();
    }

    public synchronized int pop(){
        int elem;
        while(someLock.notCondition());
        elem=(Integer)list.remove(0);
        someLock.notify()
        return elem;
    }
....
}

Upvotes: 0

JB Nizet
JB Nizet

Reputation: 692081

No, it's not thread-safe. Two threads calling push() can perfectly both read the locked as true, then add concurrently to the linked list. Since a LinkedList is not thread-safe, your code is not thread-safe.

To lock, use a lock, not an AtomicBoolean.

Upvotes: 2

Related Questions