CODEWITHSUNDEEP
javamultithreading
ward
ward

Reputation: 31

Something about multithreads with singleton

I used a singleton to read configuration:

Class Property {
      private String username;
      private String password;
      private static Property props;

      public String getUsername() {
          return username;
      }
      public String getPassword() {
          return password;
      }

      public static Property getInstance() {
          if(props == null) {
          synchronized(Property.class) {                
                   props = new Property();
                   props.initial();
              } 
          }
          return props;
      }

      public void initial() {
          Properties prop = new Properties();
          prop.load(new FileInputStream(new File("application.properties")));
          username = prop.getProperty("username");
          password = prop.getProperty("password");
      }
}

Then, in the first thread, I got an instance of Property, like props = Property.getInstance

I called the method getUsername() and getPassword() like this:

props.getUsername()/props.getPassword().

However, these two methods return null. And in the second thread and the threads after, I can get username and password from these two methods.

I don't know why this is happening. Could anyone help me with this?

Upvotes: 2

Views: 92

Answers (4)

xpa1492
xpa1492

Reputation: 1973

What both antonis_wrx and Andy N. are saying is that your synchronization is not "wide" enough:

1: if(props == null) {
2:     synchronized(Property.class) {                
3:         props = new Property();
4:         props.initial();
5:     } 
6: }

Think of 2 threads executing in parallel. Thread A executes line 1 and props == null returns true. Right after this, the OS switches to Thread B. Thread B also executes the same line 1 and the check still returns true (props is still null). Now both threads will continue and execute the synchronized block.

So say Thread A acquires the monitor on Property.class first (line 2) and initializes the props (lines 2-5). Now Thread B is free to also acquire the monitor in line 2 - it executes line 3 and at this very moment, Thread C calls getInstance().getUsername().

Thread C's getInstance executes line 1 which returns false and then returns props (which was just constructed by Thread B to new Property()). Calling getUsername() on this un-initialized Property object gives you the null you are seeing.

Note that for this example, Thread C might as well be the same Thread A (I just wanted to make it easier to read).

You can solve the problem with what has already been suggested (synchronizing the whole getInstance method). An alternative approach is to have a static block which does the initialization for you like this:

Class Property {
    private String username;
    private String password;
    private static Property props;

    static {
        props = new Property();
        props.initial();
    }

    ...
}

Upvotes: 0

alampada
alampada

Reputation: 2409

I assume that props.getProperty in the code above is a typo while posting the question.

Before the synchronized statement you check props == null. It's possible that props has been created (new Property), so not null, but the call to initial has not completed yet. So, the object returned by getInstance is not properly initialized yet before calling getUsername/password. The simplest solution would be to make the whole method synchronized but that might be a performance issue.

Also, with the current implementation, you might end up creating two instances of the singleton. E.g. if props == null and eventually both threads make it into the synchronized block. You should check if props is null in the synchronized section as well to avoid creating two instances (see: http://en.wikipedia.org/wiki/Double-checked_locking)

Probably the more elegant way to handle multi-threaded singletons is the Initialization on demand idiom (http://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom).

Upvotes: 0

Andy N.
Andy N.

Reputation: 11

"Props" are no longer null after new Property() has been executed. So props may be returned even if the initial()-method was not fully executed.

Please try the following code:

public static Property getInstance() {
      synchronized(Property.class) {                
      if(props == null) {
               props = new Property();
               props.initial();
          } 
      }
      return props;
  }

Upvotes: 1

ashokramcse
ashokramcse

Reputation: 2861

Change your static props to prop at initial() method

public void initial() {
    Properties prop = new Properties();
    prop.load(new FileInputStream(new File("application.properties")));
    username = prop.getProperty("username");
    password = prop.getProperty("password");
}

Upvotes: 4

Related Questions