0

I am not quite sure about how to use a synchronized block correctly. I know that using the synchronized keyword locks the whole class. So that is not want I want. I want to lock the single methods within the class. Here is an example from my class.

//class does only contain static methods
public class MessageFactory implements IValues {

  public static Message createRegisterRequest(String name, String password) {
      Payload payload = createPayload(name, password);
      return new Message(VALUE_REGISTRATION, payload);
  }
}

How do I correctly use a synchronize block to lock the whole method? Can I choose any parameter (or name or password)? Is there any difference between:

  public static Message createRegisterRequest(String name, String password) {
     synchronized(name){
         Payload payload = createPayload(name, password);
         return new Message(VALUE_REGISTRATION, payload);
      }
  }

and:

  public static Message createRegisterRequest(String name, String password) {
     synchronized(password){
         Payload payload = createPayload(name, password);
         return new Message(VALUE_REGISTRATION, payload);
      }
  }

and:

  public static Message createRegisterRequest(String name, String password) {
     Object lock = new Object();
     synchronized(lock){
         Payload payload = createPayload(name, password);
         return new Message(VALUE_REGISTRATION, payload);
     }
  }

createPayload is not accessing anything shared. It looks like:

 private static Payload createPayload(int playerID, String roomName){
    Payload payload = new Payload();
    payload.setPlayerID(playerID);
    payload.setRoomName(roomName);
    return payload;
}

I thought that I have to synchronize the methods as there a multiple instances accessing the methods within MessageFactory at the same time. Please let me know if I am wrong.

mollwitz
  • 213
  • 3
  • 15
  • 3
    Why do you need to synchronize at all? Is `createPayload()` accessing something shared? Just guessing, and that’s not good enough for giving you a good answer. – Ole V.V. Apr 02 '16 at 12:09
  • Thanks. So how do I lock a method that has only elementary data types as parameters? – mollwitz Apr 02 '16 at 12:09
  • `synchronized(name)` and `synchronized(password)` are broken in the same way but their unpredictable behavior might have slight differences. `Object lock = new Object(); synchronized(lock) …` has no effect at all. – Holger Apr 02 '16 at 12:11
  • 2
    You have an entirely wrong understanding of `synchronized`. It does not lock anything. It only guarantees that code synchronizing on the same object is executed by at most one thread. – Holger Apr 02 '16 at 12:13
  • @OleV.V. I edited my question. May be that you are right and there is no synchronized needed at all. I posted my concern. – mollwitz Apr 02 '16 at 12:14
  • Where/what is the mutable state you need to protect? – NilsH Apr 02 '16 at 12:15
  • You only need to care about shared mutable *data* that might be accessed by different threads concurrently. – Holger Apr 02 '16 at 12:16
  • Having multiple threads calling the same method concurrently isn't a problem at all. It's actually desired, otherwise your server would be much too slow. Problems happen when the concurrently executed code modifies state that is shared between threads. That's not the case here: there is no shared state at all. – JB Nizet Apr 02 '16 at 12:17
  • Thank you for your help. – mollwitz Apr 02 '16 at 12:18

1 Answers1

0

Synchronization in Java is made using shared object. Therefore you should have something outside the method to synchronize on. Any local variable or parameter will not work, because that data is local for every method call and is not shared.

You should do something like that:

public class MessageFactory implements IValues {
    private static final Object LOCK = new Object();

    public static Message createRegisterRequest(String name, String password) {
        synchronized(LOCK){
            Payload payload = createPayload(name, password);
            return new Message(VALUE_REGISTRATION, payload);
        }
    }
}
Vasily Liaskovsky
  • 2,248
  • 1
  • 17
  • 32