Clear Singleton instance in Java

Question

I have a Singleton class to save the state of an application's module. This class simply have a lot of class variables with setters and getters :

public class ModuleState{

private static ModuleState instance;

private A a;
private B b;
private C c;
..
..
..
..

private ModuleState (){}

public ModuleState getInstance(){
    if(instance==null)
        instance=new ModuleState();

    return instance;
}

}

At a precise moment of the application lifecycle, i have the need to CLEAR the module's state. What i do now is to reset ALL the variables in ModuleState by a clearAll() method like this:

public void clearAll(){
    a=null;
    b=null;
    c=null;

    ..
    ..
}

My question is the following : there is a cleaner method to do this reset? Possibly clearing the singleton instance itself, without resetting every class variable?

The problem with this approach is that i may have the need to add a new class variable to the ModuleState. In this case i must remember to add a line in the clearAll() method to reset the new variable.

Solution 1

What about ...

public static volatile ModuleState instance = null;

public static void reset() {
    instance = new ModuleState();
}

p.s.: as per discussion below: in a multithreaded environment it's very important to synchronize the access on the instance because the JVM is allowed to cache its value. You can use volatile as shown above. Thanks to all!

Cheers!

Solution 2

no, this approach is perfectly acceptable. you are of course synchronizing access to these state objects in some way, right? otherwise you risk someone seeing a half-cleared config object.

another thing you could do to future-proof yourself against any extra state added in the future is store all of your state in a HashMap, for example, instead of individual fields. this way, clear()ing the hashmap ensures that all state is wiped and adding any extra state in the future becomes safer

Solution 3

You need to maintain the same object instance, in order to comply with the Singleton pattern, so your approach makes sense: altering the members.

However, if you wanted to clean it up a little bit, why not just have an internal list, like:

 ArrayList<Object> members = new ArrayList<Object>();
 // If it actually is Object, there's no need to paramaterize.
 // If you want, you can actually make the members implement a common interface,
 // and parameterize the ArrayList to that.

Another Option would be to have a HashMap, that binds the key word to the member.

 HashMap<String,Object> members = new HashMap<String,Object>();
 // Again, same parameterization rules apply.

For an ArrayList or a HashMap, the clearAll method might look like this:

public class ModuleState()
{
    public void clearAll()
    {
          members.clear();
    }
}

This method won't need to change.

Comments

[JB Nizet]

The singleton wouldn't be a singleton anymore. If some code caches the result of getInstance() somewhere, it would then access or modify the previous version of the state. That has to be taken into account before choosing this solution.

[Trinimon]

Yes, you are right: it's a bit against the idea of a Singleton. In addition it might be necessary to synchronize this method. But still functional, isn't it?

[JB Nizet]

It has to cache it somehow. Just calling ModuleState.getInstance().getA().getFoo() has a chance of accessing a stale state if another thread changes the singleton in parallel. Making getInstance() synchronized doesn't change anything.

[Andrew Martin]

Could that be solved just by converting the entire class to an enum? Then, there only would be one instance type and it would be synchronised.

[eldris]

The instance variable should be declared final really, but that does prevent lazy instantiation. A reset method should clear all values within the instance, so anything using the instance are using the reset version.

[Vishy]

IMHO, the only safe singleton is a stateless one.

[Trinimon]

It's no common practice to declare the variable final. You can do this but it's not required (and it avoids lazy loading as you mentioned). Main feature of a Singleton is to have only one instance - that's still the case. Only issue to care about is access from different threads - however this can be solved by using synchronized or volatile.

[Marko Topolnik]

@JBNizet The same unsafety argument can then apply to individual objects returned by getA..Z, so obviously some practical limit to "precision" must be accepted. Making instance volatile and using this approach is probably a very decent solution to OP's problem.

[JB Nizet]

@MarkoTopolnik agreed. I wasn't saying this solution was inherently bad and didn't downvote. I was just mentioning that it could have serious side-effects if the singleton was cached, which is often the case if the singleton is a real singleton. clearijg the state of the singleton at least allows for encapsulation and non-staleness. Replacing the singleton by another one doesn't allow that.

[Marko Topolnik]

@JBNizet The only ultimately safe solution is a massive class which never exposes any of A..Z objects, but can only be queried for the leaf atomic values they hold. The whole class would have to be synchronized so that the reset operation is mutually exclusive with each get.

[JB Nizet]

Yes indeed. Or A..Z should themselves be atomic values, i.e. immutable objects.

[Woody]

it seems like the singleton pattern is perhaps not the best fit here. if you have an object who's state you need to completely reset that sounds like the kind of thing you have constructors for. I would need to see a solid reason for introducing the above code which IMO is overly complicated.

[Marko Topolnik]

@Woody Actually it doesn't achieve anything more than the simpler approach, it just allows the client to cache the "singleton". But there is no intrinsic value in having that kind of protocol: what's wrong with insisting that the singleton be always acquired from getInstance?

[Nameless]

@Marko Topolnik Can you give an example of the "only ultimately safe solution" ? thanks

[Nameless]

@MarkoTopolnik Did you mean this when you talked about "a massive class which never exposes any of A..Z objects, but can only be queried for the leaf atomic values they hold. The whole class would have to be synchronized so that the reset operation is mutually exclusive with each get" ? By the way i like this solution, any downside? Is this safe?