Commentaires

1. Le lundi 23 juillet 2012, 06:51 par mimah

Tester la non nullité de INSTANCE dans le constructeur et lever une exception si non null.

2. Le lundi 23 juillet 2012, 07:48 par Corwin

You can protect your class by enabling SecurityManager in your JVM (with option -Djava.security.manager) this solution implies no code change or you can add it programmaticaly with
"System.setSecurityManager(new SecurityManager());"
With a security manager set up, you won't be able to override access permission with reflection.

3. Le lundi 23 juillet 2012, 08:18 par Alex

private Singleton() {

       if (INSTANCE != null) {
           throw new IllegalAccessError();
       }
   }

4. Le lundi 23 juillet 2012, 08:22 par Benoît

On peut jeter une exception dans le constructeur, dès que INSTANCE n'est pas null.

5. Le lundi 23 juillet 2012, 08:25 par GroselyneBachelot

private Singleton() {

   	if( Singleton.INSTANCE != null ) {
           throw new InstantiationError( "Creating of this object is not allowed." );
       }

}

6. Le lundi 23 juillet 2012, 09:50 par fcamblor

Adding in the constructor :
if(INSTANCE != null){

   throw new IllegalStateException("Beware : instantiating twice a singleton !");

}

Would it make the deal ?

I don't really like throwing exceptions in constructors since we don't really know if object is effectively created or not... but it could be an indication for the black wizard willing to instantiate several singletons !

7. Le lundi 23 juillet 2012, 10:29 par xav

en rajoutant le lancement d'une exception dans le constructeur ?

8. Le lundi 23 juillet 2012, 13:47 par Mathieu

Une méthode peut-être en supprimant le nullary constructor, comme indiqué dans la javadoc :

public class Singleton {

   private static final Singleton INSTANCE = new Singleton(null);

   public static Singleton getInstance() {
       return INSTANCE;
   }

   private Singleton(Void v) {
   }

   public void sayHello() {
       System.out.println("Hello World !");
   }

}

Ce qui provoque une java.lang.InstantiationException lors de l'instanciation par introspection.

9. Le lundi 23 juillet 2012, 15:24 par christophe

J'ai rajouté une exception dans le constructeur s'il était déjà initialisé.

private Singleton() {

   synchronized (this.getClass()) {
       if (INSTANCE != null) {
           throw new IllegalStateException();
       }
   }

}

Par contre je doute qu'on puisse lutter contre de l'injection avec cglib et compagnie.

10. Le lundi 23 juillet 2012, 20:05 par Wouter Coekaerts

1. Make it an enum. Enum singletons FTW. (You didn't translate the note about it not being allowed in English; so for me it's still allowed because I don't understand Fren... oh, oops).

2. -Djava.security.manager

3. Check your caller:

   private Singleton() {
       if (! Singleton.class.getName().equals(new Throwable().getStackTrace()[1].getClassName())) {
           throw new SecurityException();
       }
   }

4. Rely on it that classes only get loaded once, and failing to load a class throws a different exception the first time (ExceptionInInitializerError) than attempts to use it afterwards (NoClassDefFoundError).

public class Singleton {

   ....
   
   private Singleton() {
       try {
           new Meh();
       } catch (ExceptionInInitializerError e) {
       }
   }

}

class Meh {

   static int i = 1 / 0;

}

11. Le lundi 23 juillet 2012, 20:48 par Wouter Coekaerts

If you try to prevent this by only making the constructor of Singleton throw an exception,
then it can be circumvented with the SilentObjectCreator from http://www.javaspecialists.eu/archi... .

Here's an attempt at protecting against that.

We make the Singleton class abstract so no instances of it can be created directly.
There is one subclass (SingletonImpl) that just creates one instance of itself, and then deliberately fails to load by
throwing an exception so it cannot be accessed using reflection anymore either.
That puts us in the awkward situation that the (only) instance of the singleton is of a class that failed to load.

public abstract class Singleton {

   static class SingletonImpl extends Singleton {
       private static Singleton unused = new SingletonImpl();
       private static int failToLoad = 1/0;
   }

   private static Singleton INSTANCE;

   public static Singleton getInstance() {
       if (INSTANCE == null) {
           try {
               new SingletonImpl();
           } catch (Throwable e) {
           }
       }
       return INSTANCE;
   }

   private Singleton() {
       INSTANCE = this;
   }

   public void sayHello() {
       System.out.println("Hello World !");
   }

}

12. Le lundi 23 juillet 2012, 21:55 par Laymain

C'est sale, très sale... Mais c'est une possiblité
https://gist.github.com/3166085

13. Le lundi 23 juillet 2012, 22:47 par Olivier Croisier

Il est possible de mettre INSTANCE à null par réflexion avant d'appeler le constructeur ; la solution qui coniste à vérifier la nullité du singleton est donc invalide.
En ce qui concerne le SecurityManager et les Enums, ils ne répondent pas à la question, qui consiste à sécuriser le Singleton existant et pas à modifier son environnement.

It is possible to set INSTANCE to null before calling a constructor ; therefore, checking that the instance is null is not enough.
About the SecurityManager and reimplemening the Singleton as an Enum : that's not a valid answer, as it changes the whole environment instead of securing the given Singleton.

14. Le lundi 23 juillet 2012, 22:58 par Adrien

J'ai ajouté une variable statique initialisée après la première instanciation. Le constructeur vérifie que la variable n'est pas encore initialisée.

   private static final Singleton INSTANCE = new Singleton();
   private static Boolean check= true;

(...)

   private Singleton() {
   	if (check==Boolean.TRUE) throw new RuntimeException("Merci de passer par getInstance"); 
   }

15. Le lundi 23 juillet 2012, 23:06 par Olivier Croisier

@Wouter and @Laymain : well done :)
Heinz found the answer too, and objected that one can create instances without calling a constructor (see the mentioned Javaspecialist's Newsletter), or by injecting a whole new constructor into the class (I didn't even think that could be done. This is both very ugly and exciting !)

Anyway, this quiz was meant to be a refresher about when static variables are instanciated, so Heinz's and Wouter's solutions will be considered as statistical aberrations and not common code, that do not invalidate the point.

16. Le lundi 30 juillet 2012, 12:45 par Wouter Coekaerts

Sorry if this traumatizing your yougest readers, but this breaks your Singleton: You can change the content of the String "newInstance0" to be "<clinit>". (When it's been running for some time stuff gets optimized, so also need to do this for "newInstance"). This needs to happen at just the right time: after that part of the stacktrace is created, but before the Singleton constructor checks it. So doing this concurrently in a separate thread. It needs a number of attempts; on my machine average +/- 250 or so.

import java.lang.reflect.*;

public class SingletonBreaker {

   public static void main(String[] args) throws Exception {
       Constructor<Singleton> constructor = Singleton.class.getDeclaredConstructor();
       constructor.setAccessible(true);

       // let the code warm up first. without this hacking the strings may make some class loading fail
       for (int i = 0; i < 10000; i++) {
           try {
               constructor.newInstance();
           } catch (InvocationTargetException e) {
           }
       }

       // in a background thread change the content of the Strings "newInstance0" and "newInstance" to "<clinit>"
       new Thread() {
           @Override
           public void run() {
               while (true) {
                   hackIt((Character.toString('n') + "ewInstance0").intern());
                   hackIt((Character.toString('n') + "ewInstance").intern());
               }
           }
       }.start();

       // and in parallel keep trying to call the constructor
       for (int attempt = 0; ;attempt++) {
           try {
               constructor.newInstance();
               Singleton newSingleton = constructor.newInstance();
               newSingleton.sayHello();
               System.out.println("Succeeded in attempt " + attempt);
               System.exit(0);
           } catch (InvocationTargetException e) {
           }
       }

   }

   public static void hackIt(String str) {
       try {
           final Field valueField = String.class.getDeclaredField("value");
           valueField.setAccessible(true);
           final Field countField = String.class.getDeclaredField("count");
           countField.setAccessible(true);

           valueField.set(str, "<clinit>".toCharArray());
           countField.set(str, 8);
       } catch (Exception e) {
           throw new RuntimeException(e);
       }
   }

}

17. Le lundi 30 juillet 2012, 20:35 par Olivier Croisier

Haha, nicely done :)

18. Le mercredi 1 août 2012, 15:13 par HollyDays

Perhaps I'm missing something, but it looks to me that the following Singleton is properly protected against both Wouter's nice'n'tricky trick and Heinz's deserialization ruse:

public class Singleton {

   private static final Singleton INSTANCE   = new Singleton();
   private static final String    CLASS_NAME = Singleton.class.getName();

   public static Singleton getInstance() {
       return INSTANCE;
   }

   private Singleton() {
       if (CLASS_NAME != null) {
           throw new IllegalStateException("You shall not instantiate the Singleton twice!");
       }
  }

   public void sayHello() {
       System.out.println("Hello World!");
   }

}

And if indeed I'm not mistaken, here's some sugar on the cake: the code looks much cleaner to me than the one, which tries and checks the method name of the constructor caller using an ad hoc exception!

19. Le jeudi 2 août 2012, 10:50 par HollyDays

Here's a funny thing by the way: after posting the above text yesterday, I realized while waking up this morning (no comment please ;-) ) that there was a way to set static final fields after their initialization (via reflection). Heinz published it here: http://www.javaspecialists.eu/archi... (see his class ReflectionHelper)

So I updated Wouter's SingletonBreaker class to have it try and change my above static final field CLASS_NAME (setting to null, obviously), hoping it would break my protected Singleton. I put the additional instruction directly in the main() code, not in the inner thread code, of course (right before the endless loop). And you know what? It looks like working, but actually it does not at all!

I make myself clearer: printing out the CLASS_NAME field in the SingletonBreaker shows that calling the ReflectionHelper does indeed reset this static final field to null. So the Singleton must be broken, mustn't it? But if you make the Singleton constructor print out the CLASS_NAME field value as well, you'll see that it never changes and is always non-null! So the Singleton is still not broken! And the program shows two different values for the same static field of the same class at the same time (Oops! Is Schrödinger's ghost getting closer?)

I thought at first it could be due to some weird memory-barrier side effects on accessing the same field (cf. memory implications of the Java synchronization), but trying to synchronize access to the CLASS_NAME field (on the Singleton class) led me nowhere so far. I also tried to get the object IDs of the Singleton class and its static field in memory, but I could not get any evidence that there were two different Singleton classes and/or two different CLASS_NAME fields in memory at the same time. Any more ideas?

20. Le dimanche 5 août 2012, 13:18 par Heinz Kabutz

Just a point of clarification. My "trick" does not really have anything to do with deserialization, although the deserialization uses the same trick that I do. You can also create a new constructor at run time for your class. Exercise left as an exercise to the reader. The trick with creating a new constructor instance for your class also works in Android, btw.

21. Le mercredi 8 août 2012, 14:55 par Sébastien Lorber

We can also create a different classloader with no default parent which will reload the class.

22. Le jeudi 9 août 2012, 16:16 par Heinz Kabutz

Actually, Sébastien, that's a good point. I did that once, many years ago, to enable us to invoke the JavaDoc tool several times from within one JVM. It was not designed like that, so I invoked it each time in its own classloader. Worked like a charm.

23. Le jeudi 9 août 2012, 22:24 par Olivier Croisier

Technically, using another classloader would produce a Singleton of a different class, the class' ID being comprised of the class' fully qualified name and its classloader.