3

I was writing my own implementation of BlockingQueue for practice. I am trying to avoid using the synchronized keyword for the methods. I would instead like to use ReentrantLock.

What is the best way to write this implementation? I am not a Java ninja and would greatly appreciate if someone could pinpoint the errors in my code here and suggest better ways of implementing it.

public class MyBlockingQueue<T> {

private Queue<T> queue;
private AtomicInteger limit = new AtomicInteger(10);
private Lock put_lock = new ReentrantLock();
private Lock take_lock = new ReentrantLock();
private Condition put_condition = put_lock.newCondition();
private Condition take_condition = take_lock.newCondition();

public MyBlockingQueue(AtomicInteger limit){
    queue = new LinkedList<T>();
    this.limit = limit;
}

public boolean put(T item) throws InterruptedException{
    put_lock.lockInterruptibly();
    try {
        while(queue.size() == limit.get()) {
            put_condition.await();
        }
        put_condition.signal();
        queue.add(item);
    } finally{
        put_lock.unlock();
    }

    return true;
}

public T take() throws InterruptedException{
    take_lock.lockInterruptibly();
    try {
        while (queue.size() == 0) {
            take_condition.await();
        }
        take_condition.signal();
        return queue.poll();
    } finally {
        take_lock.unlock();
    }
}

Thanks for your time!

Hir
  • 83
  • 1
  • 7
  • 1
    I think the best way is reading jdk source. do you code really work ? i see many newCondition() and then await. the ptr of condition is not saved, thus, no way to awake them. – farmer1992 Jan 19 '13 at 08:34
  • I see your point. Sure I need to save the condition first and then call await() and signal() on it. – Hir Jan 19 '13 at 08:39
  • you should first check whether code works correctly according to the promise. Check all concurrent flows. And then if you find it is not performing you can change the code. – Narendra Pathai Jan 19 '13 at 08:46
  • 1
    You question, as it stands, is more suitable for http://codereview.stackexchange.com/ – NPE Jan 19 '13 at 09:24
  • `put` and `take` use different locks and condition. As a result, they cannot interact. Meanawile, they should. – Alexei Kaigorodov Jan 19 '13 at 09:25

2 Answers2

1

You can compare your logic with open jdk implementation of Blocking queue.

ArrayBlockingQueue

Btw..ArrayBlockingQueue also uses ReentrantLock

rai.skumar
  • 10,309
  • 6
  • 39
  • 55
  • Yeah I had a look the implementation of LinkedBlockingQueue in jdk and wrote above code. – Hir Jan 19 '13 at 18:37
  • @NPE thanks for the suggestion. I asked my question at codereview.stackexchange.com – Hir Jan 19 '13 at 18:45
0

Here is the sample implementation of a Blocking Queue using producer-consumer problem analogy.

import java.util.*;
import java.util.concurrent.*;
import java.util.concurrent.locks.*;

public class MyBlockingQueue {
    Queue<Integer> myBlockingQueue = new LinkedList<>();
    ReentrantLock lock = new ReentrantLock();
    Condition added = lock.newCondition();
    Condition removed = lock.newCondition();
    private static final int INITIAL_COUNT = 0;
    private static final int MAX_COUNT = 5;
    int count = INITIAL_COUNT;
    public int produce(int num) throws InterruptedException {
        lock.lock();
        try {
           while (count == MAX_COUNT) {
               System.out.println("produce: await adding, count is max");
               added.await();
           }
           addData(num);
           added.signal();
        } finally {
            lock.unlock();
        }
        return 0;
    }

    private void addData(int num) {
        try {
            Thread.sleep(500);
        } catch (InterruptedException e) {
        }
        System.out.println("added data ! " + myBlockingQueue.offer(num));
        count += 1;
    }

    private void removeData() {
        System.out.println("removed data ! " + myBlockingQueue.poll());
        count -= 1;
    }

    public int consume() throws InterruptedException {
        lock.lock();
        try {
            while (count == 0) {
                System.out.println("consume: await removing, count is 0");
                removed.await();
            }
            removeData();
            removed.signal();
        } finally {
            lock.unlock();
        }
        return 0;
    }

    public static void main(String[] args) throws InterruptedException {
        final MyBlockingQueue instance = new MyBlockingQueue();
        List<Callable<Integer>> callableList = new ArrayList<>();
        int n = 10;
        for (int i = 0; i < n; i += 1) {
            if (i % 2 == 0) {
                callableList.add(() -> instance.produce(new Random().nextInt(n * n)));
            } else {
                callableList.add(() -> instance.consume());
            }
        }
        ExecutorService executorService = Executors.newFixedThreadPool(4);
        List<Future<Integer>> futures = executorService.invokeAll(callableList);
        System.out.println(instance.count);
        System.out.println(instance.myBlockingQueue);
        executorService.shutdown();
        executorService.awaitTermination(2, TimeUnit.SECONDS);
    }
}
Nishant Ingle
  • 793
  • 1
  • 8
  • 12