Synchronization block inside callback method

Question

I'm using Apache mina in one of my projects. The doDecode() of CumulativeProtocolDecoder is called every time a chunk of data is received. I'm concatenating these chunks together until I get a special character at the end of the string. So I start concatenating when I receive $ as the first character and end concatenation when I receive another $ character.

I want to make the concatenation part synchronized to avoid any potential non intended concatenations.

By encapsulating the concatenating block with synchronized() clause I can make this operation thread safe but My question is while one thread is busy doing the concatenations and another thread calls doDecode() with the new data, will the new info provided as an argument to doDecode() will be lost because the synchronized block is busy or will it wait and keep the argument cached until the synchronized block is available again?

@Override
    protected boolean doDecode(IoSession ioSession, IoBuffer ioBuffer, ProtocolDecoderOutput protocolDecoderOutput) throws Exception {
        System.out.println("inside decoder");

        try {
            IoBuffer data = (IoBuffer) ioBuffer;
            // create a byte array to hold the bytes
            byte[] buf = new byte[data.limit()];

            System.out.println("REPSONSE LENGTH: "+ data.limit());
            // pull the bytes out
            data.get(buf);
            // look at the message as a string
            String messageString = new String(buf);

            synchronized (messageString) {
                //do concatenatoin operatoins and other stuff
            }

        }
        catch (Exception e) {
            e.printStackTrace();
        }
        return true;
    }

Answer 1

Synchronizing on a local variable won't do anything useful, so you can safely remove that block.

Every thread calling doDecode will have its own copy of method arguments, so you can be safe that no argument will be changed in between.

I'm guessing concatenating these chunks means storing them in some member field of your Decoder class.

In this case, you probably want to synchronize on a field. E.g:

private final Object lock = new Object();

@Override
protected boolean doDecode(IoSession ioSession, IoBuffer ioBuffer, ProtocolDecoderOutput protocolDecoderOutput) throws Exception {

    // ...
    synchronized (this.lock) {
        // do concatenation operations and other stuff
    }
    // ...
}

---

I'm just not sure whether it's good practice to synchronize within a framework component that it's probably meant to handle requests concurrently.