Do I have to synchronize access to encapsulated thread-safe data structures in Java?

Question

Say I have something like this (and I do)

class QueBean extends JPanel {
    private Queue queue = new LinkedBlockingQueue();

    public Object poll(){
        return queue.poll();
    }
}

with some of these that run on their own threads

class ConsumerBean extends JPanel implements Runnable{
    private QueBean queBean;

    public synchronized run(){
        while (true) {
           Object result =  queBean.poll();
           if (result != null) {
              jResultTextField.setText("got one");  
           }
           wait(500);
        }
    }
}

Should my poll() in the QueBean be synchronized or not?

Answer 1

No. There is no need. Since your poll method does nothing except call a thread-safe method, there is no possibility of data corruption.

Answer 2

You don't need to do this, so long as queue does not change in QueBean.

Also, unless you're trying to implement some kind of trivial rate-limitng, you don't need the wait(500) in your code. It's superfluous due to the queue being blocking.

Answer 3

There is a threading problem, but not the one you think--The code you posted is almost certainly illegal and will eventually lock up.

One of the core rules of Swing is that only one thread is allowed to touch "realized" components. (Realized means on-screen or "almost" on-screen).

This:

jResultTextField.setText("got one"); 

Inside a thread is pretty sure to be wrong--you just can't do it. Check out invokeLater or invokeAndWait to get your screen updates onto your AWT thread.

By the way--it feels funny having threads in anything that extends a component--seeing that lead me to IMMEDIATELY search for where the conflict was, but it should make any long-time Java programmer uneasy at a glance--I suggest you split up your classes some and completely separate the part that drives your GUI (Controller) from the GUI (View)..

Answer 4

External synchronization is not necessary in this case. Read the BlockingQueue contract:

BlockingQueue implementations are thread-safe. All queuing methods achieve their effects atomically using internal locks or other forms of concurrency control.