Single static field in Java class

Question

There is a class RedisLogger.java which used to deal with logger with redis. In RedisLogger.java, I declared a static JedisPool field jedisPool using this code:

private static JedisPool jedisPool;

Cause JedisPool is thread-safe calss, I want to instantiate jedisPool only once in my application by using the following code:

public static JedisPool getJedisPool() {
    if(jedisPool == null) {
        synchronized (JedisPool.class) {
            if(jedisPool == null) {
                jedisPool = new JedisPool();
            }
        }
    }
    return jedisPool;
}

I used this code to test it.

ExecutorService executor = Executors.newCachedThreadPool();
    for(int i = 0; i < 1000; i++) {
        executor.execute(()->{
            System.out.println(RedisLogger.getJedisPool());
        });
    }

It seems works well from output :

redis.clients.jedis.JedisPool@3d11fc5d
redis.clients.jedis.JedisPool@3d11fc5d
redis.clients.jedis.JedisPool@3d11fc5d
redis.clients.jedis.JedisPool@3d11fc5d
redis.clients.jedis.JedisPool@3d11fc5d
redis.clients.jedis.JedisPool@3d11fc5d
redis.clients.jedis.JedisPool@3d11fc5d
redis.clients.jedis.JedisPool@3d11fc5d
redis.clients.jedis.JedisPool@3d11fc5d
redis.clients.jedis.JedisPool@3d11fc5d
redis.clients.jedis.JedisPool@3d11fc5d
redis.clients.jedis.JedisPool@3d11fc5d
redis.clients.jedis.JedisPool@3d11fc5d
....

But can it really achieve my expectations? Becasue there are many places like this in my application. for example.

private static Cluster getCluster() {
    if(cluster == null) {
        synchronized (Cluster.class) {
            if(cluster == null) {
                Builder builder = Cluster.builder();
                for (int i = 0; i < MSConfig.SRCDOC_CASSANDRA_ADDRS().length; i++) {
                    builder.addContactPoint(MSConfig.SRCDOC_CASSANDRA_ADDRS()[i])
                            .withPort(MSConfig.SRCDOC_CASSANDRA_PORT()[i]);
                }
                cluster = builder.withCredentials(MSConfig.SRCDOC_CASSANDRA_USERNMAE(), MSConfig.SRCDOC_CASSANDRA_PASSWORD())
                        .withProtocolVersion(ProtocolVersion.V4)
                        .withPoolingOptions(getPoolingOptions())
                        .withSocketOptions(getSocketOptions())
                        .withRetryPolicy(getRetryPolicy())
                        .withQueryOptions(getQueryOptions())
                        .build();
            }
        }
    }
    return cluster;
}

Thanks!!!

*But can it really achieve my expectations*. It should. Why shouldn't it? Do you see inconsistencies? We don't know your code, so we can't tell by looking at your last snippet if it will work. We would need the whole picture, but that would burst the frame of this question — Lino, Aug 27 '18 at 07:02

Lino · Answer 1 · 2018-08-27T07:12:18.560

The only addition I think of is to drop that synchronized initialization and just directly go with the static-holder-pattern. From the top answer:

The JVM defers initializing the InstanceHolder class until it is actually used, and because the Singleton is initialized with a static initializer, no additional synchronization is needed.

This can be used in your code like this:

public class RedisLogger{
     public static JedisPool getJedisPool(){
         return JedisPoolHolder.INSTANCE;
     }

     private static final class JedisPoolHolder{
         private static final JedisPool INSTANCE = new JedisPool();
     }

     // the rest of your code
}

score 1 · Answer 2 · answered Aug 27 '18 at 07:06

1

What you are doing is called "double checked locking". If you search this on Stackoverflow or Google you will find many explanations why it does not work properly in Java.

The alternatives are:

If it is very very likely that you will use this object when its holding class was loaded, then just initialize it directly and make the field final.
Declare the field volatile.
Use a holding class, i.e., have a private inner class that holds the field, which is then made final.

See also these questions:

answered Aug 27 '18 at 07:06

Cephalopod

14,632
7
51
70

I know，double checked is used to Singleton pattern. But I want use it to a field. – xielei Aug 27 '18 at 07:11
The code you currently have can leak an uninitialized instance of `JedisPool`. This has nothing to do with singletons. – Cephalopod Aug 27 '18 at 07:13

score 0 · Answer 3 · edited Aug 27 '18 at 07:02

0

You can directly assign a value to variable and declare it final static in your case.

edited Aug 27 '18 at 07:02

XtremeBaumer

6,275
3
19
65

answered Aug 27 '18 at 06:54

wieczorekm

158
2
9

4

`static` is the keyword that makes something a singelton. `final` only ansures, that the value of the variable can't be changed. Therefore, your answer as it stands is wrong – XtremeBaumer Aug 27 '18 at 06:56
Right, it's not necessarily about singletons but in this context final would be nice because we don't want to modify the value, so since it will be assigned during declaration, it can be final. Edited my answer though. – wieczorekm Aug 27 '18 at 06:59
1

But you will still need `static` – XtremeBaumer Aug 27 '18 at 07:00
Little misunderstanding. I meant adding final to the modifiers, not replacing them with final. Thanks for the edit. – wieczorekm Aug 27 '18 at 07:01
Since it's a pool , I suppose it will be modified sometime... i don't think `final` is right here – Leviand Aug 27 '18 at 07:03
I'm trying to used final field, but it can't because RedisLoggerl.java is util class. and there is a static function ' public static void execute(CallWithJedis caller) { try(Jedis jedis = getJedisPool().getResource()){ caller.call(jedis); } }' to uesd it. – xielei Aug 27 '18 at 07:04

Single static field in Java class

3 Answers3