36

How thread-safe is enum in java? I am implementing a Singleton using enum (as per Bloch's Effective Java), should I worry at all about thread safety for my singleton enum? Is there a way to prove or disprove that it is thread safe?

// Enum singleton - the preferred approach
public enum Elvis { 
    INSTANCE;
    public void leaveTheBuilding() { ... }
}

Thanks

Lydon Ch
  • 8,637
  • 20
  • 79
  • 132
  • What do you mean by "thread-safe"? The `leaveTheBuilding` method isn't synchronized, so of course it could be run by multiple threads simultaneously. Or are you talking about initializing the `INSTANCE`? – Michael Myers Mar 28 '10 at 04:20
  • 1
    When you say "singleton", do you mean it has mutable state? In that case you're on to a loser anyway. – Tom Hawtin - tackline Mar 28 '10 at 04:46
  • 1
    Just my $0.02, but I think using an enum to enforce the Singleton pattern is code obfuscation. That said, I know Bloch along with many others suggest it and my comment may be the barking of a dinosaur. – Tim Bender Mar 28 '10 at 06:53
  • @Tom No, I don't have any mutable state. @mmyers, I was talking about initializing the INSTANCE, not accessing the method. Of course I will need to synchronize the leaveTheBuilding method to make it thread-safe – Lydon Ch Mar 28 '10 at 12:11
  • methods in enum are not thread-safe until you make it synchronized. – Arnav Joshi Sep 25 '16 at 18:24

4 Answers4

43

As @Mike is saying, creation of enum is guaranteed to be thread safe. However, methods that you add to an enum class do not carry any thread safety guarantee. In particular, the method leaveTheBuilding may be executed, concurrently, by multiple threads. If this method has side effects (changes the state of some variable) then you need to think about protecting it (i.e., make it synchronized) or parts thereof.

Itay Maman
  • 30,277
  • 10
  • 88
  • 118
11

Customized Enum Definition may be not thread safe. For example,

RoleEnum.java:

package com.threadsafe.bad;

public enum RoleEnum {
       ADMIN(1),
       DEV(2),
       HEAD(3);

       private Integer value;
       private RoleEnum(Integer role){
              this.value=role;           
       }
       public static RoleEnum fromIntegerValue(Integer role){

              for(RoleEnum x : values()){
                     if(x.value == role ){
                           return x;
                     }
              }
              return RoleEnum.HEAD;             
       }

       Class<?> buildFromClass;
       public void setBuildFromClass(Class<?> classType){
              buildFromClass=classType;
       }
       public Class<?> getBuildFromClass(){
              return this.buildFromClass;
       }
}

Main.java:

package com.threadsafe.bad;

public class Main {

       public static void main(String[] args) {
              // TODO Auto-generated method stub

              Thread threadA = new Thread(){
                     public void run(){
                           System.out.println("A started");
                           RoleEnum role;
                           role=RoleEnum.fromIntegerValue(1);
                           System.out.println("A called fromIntegerValue");
                           role.setBuildFromClass(String.class);
                           System.out.println("A called setBuildFromClass and start to sleep");


                           try {
                                  Thread.sleep(10000);
                           } catch (InterruptedException e) {
                                  // TODO Auto-generated catch block
                                  e.printStackTrace();
                           }
                           System.out.println("Thread A: "+role.getBuildFromClass());
                     }
              };

              Thread threadB = new Thread(){
                     public void run(){
                           System.out.println("B started");
                           RoleEnum role;
                           role=RoleEnum.fromIntegerValue(1);
                           role.setBuildFromClass(Integer.class);
                           System.out.println("B called fromIntegerValue&setBuildFromClass and Start to sleep");
                           try {
                                  Thread.sleep(20000);
                           } catch (InterruptedException e) {
                                  // TODO Auto-generated catch block
                                  e.printStackTrace();
                           }
                           System.out.println("B waked up!");

                           System.out.println("Thread B: "+ role.getBuildFromClass());
                     }

              };

              threadA.start();
              threadB.start();


       }

}

Sometimes the output will be:

B started

B called fromIntegerValue&setBuildFromClass and Start to sleep

A started

A called fromIntegerValue

A called setBuildFromClass and start to sleep

Thread A: class java.lang.String

B waked up!

Thread B: class java.lang.String <-We expect java.lang.Integer

Sometimes the output will be:

A started

A called fromIntegerValue

A called setBuildFromClass and start to sleep

B started

B called fromIntegerValue&setBuildFromClass and Start to sleep

Thread A: class java.lang.Integer <-We expect java.lang.String

B waked up!

Thread B: class java.lang.Integer

Jichao Zhang
  • 436
  • 5
  • 7
9

This technique is absolutely thread-safe. An enum value is guaranteed to only be initialized once, ever, by a single thread, before it is used. However, I'm not sure whether it is when the enum class is loaded or the first time the enum value itself is accessed. Using this technique is actually a bit safer than other techniques, because there is not even a way with reflection to get a second copy of your enum-based singleton.

Mike Daniels
  • 8,582
  • 2
  • 31
  • 44
  • 1
    If the enum does something really stupid in its static intialiser (or elsewhere), it could theoretically be a problem. (Static initialisation is a different phase from class loading - see 3-argument `Class.forName`.) – Tom Hawtin - tackline Mar 28 '10 at 04:44
  • yes i tried to find a reflective way to init another enum, and i couldn't. – Lydon Ch Mar 28 '10 at 11:48
  • @Mike How can I find out when the enum value is initialized, ie when the enum class is loaded or the first time the enum value is accessed? – Lydon Ch Mar 28 '10 at 12:12
  • @portoalet, as you know, each enum value is like its own instance of an object and can have constructors and methods. I would do a little test to see when an enum value's constructor gets called. You could create a small test scenario with an enum with two values, and only try to use one of them, and see if only one enum value's constructor gets called or both of them do. – Mike Daniels Mar 28 '10 at 21:15
  • @LydonCh create constructor with private or not access modifier – Abdul Jun 26 '19 at 04:56
4

Adding synchronized avoids inconsistent state with enums.

Code below will run will lock nicely alway printing "One". However when you comment out synchronized there will be other values printed too.

import java.util.Random;
import java.util.concurrent.atomic.AtomicInteger;

public class TestEnum
{
    public static AtomicInteger count = new AtomicInteger(1);

    public static enum E
    {
        One("One"),
        Two("Two");

        String s;

        E(final String s)
        {
            this.s = s;
        }

        public void set(final String s)
        {
            this.s = s;
        }

        public String get()
        {
            return this.s;
        }
    }

    public static void main(final String[] args)
    {
        doit().start();
        doit().start();
        doit().start();
    }

    static Thread doit()
    {
        return new Thread()
        {
            @Override
            public void run()
            {
                String name = "MyThread_" + count.getAndIncrement();

                System.out.println(name + " started");

                try
                {
                    int i = 100;
                    while (--i >= 0)
                    {

                        synchronized (E.One)
                        {
                            System.out.println(E.One.get());
                            E.One.set("A");
                            Thread.sleep(new Random().nextInt(100));
                            E.One.set("B");
                            Thread.sleep(new Random().nextInt(100));
                            E.One.set("C");
                            Thread.sleep(new Random().nextInt(100));
                            E.One.set("One");
                            System.out.println(E.One.get());
                        }

                    }
                }
                catch (InterruptedException e)
                {
                    // TODO Auto-generated catch block
                    e.printStackTrace();
                }

                System.out.println(name + " ended");
            }
        };
    }
}
Bohdan
  • 16,531
  • 16
  • 74
  • 68