admin@glassfish.java.net

Re: Code Review Needed

From: Bill Shannon <bill.shannon_at_sun.com>
Date: Thu, 09 Nov 2006 16:52:59 -0800

Byron Nevins wrote:
> There is one point you made that I disagree with or just don't understand:
>
> /There seems to be no synchronization of the objects shared between the
> worker threads and the calling thread.
>
> /I still don't see a problem. we have this situation:
>
> thread1 is writing into an unsynchronized StringBuilder object (there is
> one and only one thread writing)
> thread2 is reading from the SB whenever it wants.
>
> My position is that there is nothing that either thread can do to either
> corrupt the SB object or cause any problems to the other thread. Thus
> it is perfectly safe code.
>
> If I'm wrong -- can you explain -- because this stuff comes up pretty
> regularly and I would change my practices...

You're wrong.

You don't understand the Java memory model. (And I'm not an expert in
the Java memory model, so I may not explain it correctly either.)

My understanding is that there is no guarantee that any data written
by one thread will be *visible* to the other thread unless you use
synchronization. Since StringBuilder is unsynchronized, the data
written by one thread may never be seen by the other thread.

Even ignoring the memory model issues, which may not occur on the
kinds of machines you're using for testing, there's lots of other
potential problems here. Think about what has to be done in the
StringBuffer implementation when appending data, and what might be
visible to another thread during any phase of that operation. While
it might be possible to implement StringBuffer carefully so that the
reader always sees a consistent view, does StringBuffer guarantee that
it does so? When used by a single thread, or even when within a
synchronized method, the order of updating the length of the buffer
vs. copying data into the buffer wouldn't matter. But if another
thread is racing with you to see the state of the buffer, the order
of those operations is critical. And in particular, you have to
guarantee that those two bits of state don't end up in different
cache lines that are available to the other thread in a different
order because of out of order memory fetch operations. The only
thing that guarantees that order is synchronization.

StringBuilder exists as an unsynchronized version of StringBuffer
*only* for the case where the entire use of the object is within a
single thread. The case you have is exactly the case that calls for
StringBuffer.

Note also that there were similar problems with some of the other
objects you were using.

You'll find more information on these issues in Effective Java and
other fine Java programming books. :-)