0

I have this below code. 

private static volatile Properties props = null;
    private static volatile StanfordCoreNLP pipeline = null;
    /**
     * 
     * @return
     */
    static {
        if (props == null) {
            props = new Properties();
            props.setProperty("annotators", "tokenize, ssplit, parse, sentiment");
        }
        if (pipeline == null) {
            pipeline = new StanfordCoreNLP(props);
        }
    }

I want to have a single instance of variable props and pipeline across my entire application which is multithreaded.

Is my code right or I am missing something?

Thanks.

Prakash P
  • 3,582
  • 4
  • 38
  • 66

3 Answers3

1

It's possible to get rid of the volatile by using a method call and still retain thread safety through static initialization.

private static final Properties props = initProperties();
private static Properties initProperties() {
    Properties props = new Properties();
    props.setProperty("annotators", "tokenize, ssplit, parse, sentiment");
    return props;
}

public static Properties getProperties() {
    return props;
}

Edit: To answer your question though, yes your code in OP is indeed thread safe, although what I've given before is the way that I would do it personally.

0

your code seems right as you are creating the instance at class loading time, so you only have to create the getters for them

But i recommend using the singleton pattern in this case as shown below

i have used singleton pattern with double locking for multi threaded environment

public class Utility{

        private static final Object masterLock = new Object();
        private static volatile Properties props = null;
        private static volatile StanfordCoreNLP pipeline = null;

        static Properties getPropertiesInstance(){

            if(props == null){

                synchronized (masterLock) {
                    if(props == null){
                        props = new Properties();
                    }
                }
            }

            return props;
        }

        static StanfordCoreNLP getStanfordCoreNLPInstance(){

            if(pipeline == null){

                synchronized (masterLock) {
                    if(pipeline == null){
                        pipeline = new StanfordCoreNLP();
                    }
                }
            }

            return pipeline;
        }
}
Rajiv Kapoor
  • 315
  • 1
  • 7
  • kindly make necessary changes in the 'if' condition of both synchronized blocks – Rajiv Kapoor Feb 03 '17 at 12:40
  • Yes, singleton will work. I too have used it several times. But in my above code won't new objects are creating when every thread is loading this class. – Prakash P Feb 03 '17 at 14:34
  • didn't understood about the object creation you're talking about "but in my above code won't new objects . . . . ." – Rajiv Kapoor Feb 03 '17 at 16:51
0

volatile is not needed in your case. Java guarantees that the static initialization is properly synchronized. There is also no need for a method call. Also see Are Java static initializers thread safe? and https://www.cs.umd.edu/~pugh/java/memoryModel/jsr-133-faq.html

Community
  • 1
  • 1
Jan Schaefer
  • 1,882
  • 1
  • 18
  • 23