Re: Oldie Synchronize Question from Newbie

Russ wrote:
> [...]

My question is in regard to when a method needs to be "synchronized"
I did not write the following snippets of code and have some
reservations about them.

The following method can possibly be called by two or more different
threads at the same time:

  public  void sendMessage( Message msg ) throws IOException {
    if( msg.getType() == ControlMessage.TYPE ){
      sendRawMessage( msg );
    }
    else {
         msgBuffer.put( msg );
    }
  }

By all rights then, shouldn't "sendMessage" be synchronized?

It does not appear to be anything that requires synchronisation. Presumably msgBuffer is fixed. It doesn't access anything mutable that might be shared. Two threads passing through the method (or other blocks synchronised on the instance) at the same time will notice each other.

   protected void sendRawMessage( Message msg ) throws IOException {
    synchronized( out ){
[...]
    }
  }


   public void put( Message msg ){
      synchronized( buffer ){
[...]
      }
    }
[...]
public void run(){
      Message msg;
      while( Thread.currentThread() == runner ){
        try{
          if(!buffer.isEmpty()){
            synchronized( buffer ){
              msg =(Message) buffer.firstElement();
              buffer.removeElementAt(0);
            }
            sendRawMessage( msg );
          }
          else {
            synchronized( buffer ){
               buffer.wait();
            }
          }

Here be race conditions. Almost always, wait should be called from within a while loop. To synchronised blocks like that are a good sign of trouble.

        for (;;) {
            Message message;
            synchronized (buffer) {
                for (;;) {
                    if (Thread.currentThread() != runner) {
                        return;
                    }
                    if(!buffer.isEmpty()) {
                        break;
                    }
                    buffer.wait();
                }
                message = (Message)buffer.firstElement();
                buffer.removeElementAt(0);
            }
            sendRawMessage(message);
        }

(Usual disclaimer.)

Now since "sendRawMessage" method is being called by two seperate
threads ( diversion "msgBuffer" and original "sendMessage" )
shouldn't it be synchronized as well?


What are you trying to protect?

Or am I simply off base here, because since both Vector and
DataOutputStream classes are synchronized the methods containing
them don't need to be?

In general you need to group together operations into a logical task. Your operations seem to do that on out and buffer.

Tom Hawtin
--
Unemployed English Java programmer
http://jroller.com/page/tackline/
.

Relevant Pages

  • [PATCH 12 of 28] IB/ipath - print more informative parity error messages
    ... @msgl: the size of the output buffer ... * Use same msg buffer as regular errors to avoid ... We reuse the same message buffer as ...
    (Linux-Kernel)
  • Re: Problem with socket.recv()
    ... Frank Preiswerk wrote: ... Here's an idea that's a little different from what you're asking for, but it shows a working buffer implementation. ... part = nil ... msg << part ...
    (comp.lang.ruby)
  • Re: Char array copy
    ... I've got a an unsigned char array ... After that I pass msg to a method, ... How can I pass the full buffer content to the ...
    (microsoft.public.vc.mfc)
  • Re: Line count, the best strategy?
    ... MSG wrote: ... > but in Perl Cookbook, ... B doesn't explicitly manage a buffer. ... Prev by Date: ...
    (comp.lang.perl.misc)