Synchronization required on an object which should be singleton by convention

Question

I want that only one instance of my class, Format1 should be returned through the class Engine:

 public final class Engine {

     private static Format format;

     public Engine(final Param1 param1, final Param2 param2) {
         Engine.format = new FormatV1(param1);   // FormatV1 implements Format
     }

     static Format getFormat(int condition) {
         switch(condition) {
            case 1 : return format;
         }
     }
 }

Does this ensure that only one and correct instance of Format is returned via getFormat?

Also, the client which invokes the getFormat() needs to execute method on the format object (which has absolutely no state) but passes a ByteBuffer to the method (this ByteBuffer might be modified):

 class Client 
 {
     public static void main(String[] args) {

          Format obj = Engine.getFormat(1); // some param
          ByteBuffer buffer;                // read data from disk

          synchronized (obj) {

                 long[] result = obj.method(buffer);

                 if (result == null) {
                      // do something
                 }
          }
     }
 }

Does the synchronized construct here/like this ensure serializability ?

P.S: I'm very new to Java, as much as I read I could also make the method synchronized but I think the check (result == null) should also be included in the critical section so I decided to use the synchronized (obj) construct instead. Please forgive me if this is a silly question but I want to confirm my doubts.

Answer 1

currently, you are creating the Format instance in the Engine constructor, which is most likely not what you want (your current example would throw a NullPointerException.

secondly, you have 2 synchronization concerns. first is the correct publication of the format instance. you don't do this currently. you should either use class initialization (instantiate the format instance on class create) or make the format member volatile in order to correctly publish the format instance.

secondly, if your format instance has mutable state, then you should make the FormatV1 methods themselves synchronized. making the caller synchronize on the object before using it is a very bad design.

Answer 2

You created a constructor of the Engine class to initialize the static variable, instead, create a static initializer, using synchronized on that method

public static synchronized void initFormat(....) {
   if (format == null) {
      ... create it
   } else {
   .... your chocie: throw an exception or do nothing 
   }
}