Reputation: 2552
I've just implemented a custom blocking queue with an semaphore.
for a reason i cant find, my queue isn't getting blocked by the semaphore when my queue is empty.
here's my implementation:
package poolThread;
import java.util.LinkedList;
import java.util.Queue;
import java.util.concurrent.Semaphore;
public class MyQueue<E> {
Semaphore s = new Semaphore(0, true);
private Queue<E> queue = new LinkedList<E>();
public boolean isEmpty(){
return this.queue.isEmpty();
}
public void enqueue(E e){
queue.add(e);
s.release();
}
public E dequeue(){
E e = null;
try {
s.acquire();
} catch (InterruptedException e1) {
// TODO Auto-generated catch block
e1.printStackTrace();
}
e = queue.remove();
return e;
}
}
could you help me find the fault in my code?
Upvotes: 2
Views: 848
Reputation: 2566
The problem here is the LinkedList
- which isn't thread-safe. So even if the permits are acquired properly, the remove()
operation on the LinkedList
can (and will) cause troubles. Here's a simple "test case" to show the behavior:
MyQueue<String> x = new MyQueue<>();
ExecutorService es = Executors.newFixedThreadPool(2);
for (int j = 0; j < 2; j++)
es.submit(() -> {
String tn = Thread.currentThread().getName();
for (int i = 0; i < 2; i++)
x.enqueue("v" + i);
for (int i = 0; i < 2; i++)
System.out.println(tn + " deq: " + x.dequeue());
});
The output will be something like (you will see null
s due to NoSuchElementException
s on the remove
method):
pool-1-thread-2 deq: v0
pool-1-thread-1 deq: null
The simplest solution for this would be to replace your LinkedList
with a java.util.concurrent.ConcurrentLinkedQueue
.
Upvotes: 2