Geek Answers Handbook

How to create the SynchronizedStack class correctly?

I made a simple synchronized Stack object in Java, just for training purposes. Here is what I did:

public class SynchronizedStack { private ArrayDeque<Integer> stack; public SynchronizedStack(){ this.stack = new ArrayDeque<Integer>(); } public synchronized Integer pop(){ return this.stack.pop(); } public synchronized int forcePop(){ while(isEmpty()){ System.out.println(" Stack is empty"); try { wait(); } catch (InterruptedException e) { e.printStackTrace(); } } return this.stack.pop(); } public synchronized void push(int i){ this.stack.push(i); notifyAll(); } public boolean isEmpty(){ return this.stack.isEmpty(); } public synchronized void pushAll(int[] d){ for(int i = 0; i < d.length; i++){ this.stack.push(i); } notifyAll(); } public synchronized String toString(){ String s = "["; Iterator<Integer> it = this.stack.iterator(); while(it.hasNext()){ s += it.next() + ", "; } s += "]"; return s; } }

Here are my questions:

  • Is it good to sync the isEmtpy() method? I realized that even if another thread modifies the stack at the same time, it will still return a consistent result (there is no operation that goes into isEmpty state, which is neither initial nor final). Or is it the best design to synchronize all methods of a synchronized object?

  • I don't like the forcePop() method. I just wanted to create a thread that could wait until the item was pushed onto the stack before it pushed the item, and I thought the best option is to make a loop using wait() in the run() method of the thread, but I can't because it throws an IllegalMonitorStatException . What is the right way to do something like this?

  • Any other comments / suggestions?

Thanks!

+6
java multithreading synchronization
source share
4 answers

  • Stack itself is already synchronized, so it makes no sense to apply synchronization again (use ArrayDeque if you want to execute an unsynchronized implementation of the stack)

  • This is NOT OK (except for the fact from the previous point), since the lack of synchronization can cause memory visibility effects.

  • forcePop() pretty good. Although it must pass an InterruptedException without catching it in order to follow the contract of the intermittent blocking method. This will allow you to interrupt a thread blocked by calling forcePop() by calling forcePop() Thread.interrupt() .

+7
source share

The only issue with isEmpty() being synchronized is that you don't know what happens below. Although your reasoning is reasonable, it assumes that the underlying Stack also behaves reasonably. Most likely, this is the case, but you cannot rely on it as a whole.

And the second part of your question, there is nothing wrong with blocking pop operations, see this for the full implementation of all possible strategies.

And one more suggestion: if you create a class that can be reused in several parts of the application (or even several applications), do not use synchronized methods. Do this instead:

 public class Whatever { private Object lock = new Object(); public void doSomething() { synchronized( lock ) { ... } } }

The reason for this is that you really don’t know whether users of your class want to synchronize your Whatever instances or not. If they do, they can interfere with the class itself. Thus, you have your own private castle, which no one can interfere with.

0
source share

Assuming stack.isEmpty() doesn't need synchronization, it might be true, but you are relying on the implementation detail of a class that you don't have. The javadocs Stack states that the class is not thread safe, so you should synchronize all access.

0
source share

I think you're mixing idioms a bit. You support SynchronizedStack with java.util.Stack , which in turn is supported by java.util.Vector , which is equal to synchronized . I think you should encapsulate the wait() and notify() behaivor behavior in another class.

0
source share

More articles:


All Articles