Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How do I avoid problems arising from accessing static fields before the class is initialized?

I have this code:

public abstract class Person {

    public static final class Guy extends Person {

        public static final Guy TOM = new Guy();
        public static final Guy DICK = new Guy();   
        public static final Guy HARRY = new Guy();
    }

    public static final List<Guy> GUYS = ImmutableList.of(Guy.TOM, Guy.DICK, Guy.HARRY);
}

I know, it looks like it should be an enum, and it would be, but it needs to inherit from Person, an abstract class.

My problem is: If I try to access the list of Guys, I'm okay. But it I try to access any one Guy in particular, I have a problem: Guy gets loaded before Person. However, since Guy inherits Person, Person gets loaded, and tries to access TOM, but since Guy is already being loaded, it can't start to load again, and so we have a null reference. (And ImmutableList doesn't accept null, so we get an exception.)

So I know why this is happening, but I'm not sure how to avoid it. I could move the List into the Guy class, but I'll have more than one implementation of Person, and I'd like to have a master list of all Persons in the Person class.

It seems strange to me that it's possible to load a static inner class without loading its containing class, but I guess it makes sense. Is there any way to solve this problem?

like image 699
codebreaker Avatar asked May 14 '15 23:05

codebreaker


2 Answers

You could keep your immutable list in a separate class, like People.GUYS:

public class People {
    public static final List<Guy> GUYS = ImmutableList.of(Guy.TOM, Guy.DICK, Guy.HARRY);
}

This way you can still keep the individual guys in the Guy class:

public abstract class Person {

    public static final class Guy extends Person {

        public static final Guy TOM = new Guy();
        public static final Guy DICK = new Guy();   
        public static final Guy HARRY = new Guy();
    }
}
like image 87
Dmitry Grigoryev Avatar answered Sep 20 '22 16:09

Dmitry Grigoryev


A solution for the sake of solution, this seems working (for single thread)

public class Person
{
    static Lazy tom = new Lazy();
    static Lazy cat = new Lazy();

    public static final List<Guy> GUYS = ImmutableList.of(tom.get(), cat.get());

    public static final class Guy extends Person
    {
        public static final Guy TOM = tom.get();
        public static final Guy CAT = cat.get();
    }

    static class Lazy
    {
        Guy v;
        Guy get()
        {
            if(v==null)
            {
                Object x = Guy.TOM; // just to trigger Guy initialization
                if(v==null)
                    v = new Guy();
            }
            return v;
        }
    }

OP's design is inherently recursive. Although Java has a well-defined initialization procedure (which backs off when recursion is detected), the behavior varies depending on which class is initialized first.

The problem is worsened by the requirement that the fields are final, which severely limits how we can assign them. Otherwise, we can try if(null) assign in multiple places to solve the problem.

The solution posted above essentially uses a temporary variable to workaround the final constraint.


A modified version using a Map, suitable for more number of fields:

public class Person
{
    static Map<String,Guy> cache = new HashMap<>();  // remove after both class are initialized
    static Guy get(String name)
    {
        if(!cache.containsKey(name))
        {
            Object x = Guy.TOM; // just to trigger Guy initialization
            if(!cache.containsKey(name))
                cache.put(name, new Guy());
        }
        return cache.get(name);
    }

    public static final List<Guy> GUYS = ImmutableList.of(get("tom"), get("cat"));

    static{ if(Guy.TOM!=null) cache=null; }

    public static final class Guy extends Person
    {
        public static final Guy TOM = get("tom");
        public static final Guy CAT = get("cat");

        static{ if(Person.GUYS!=null) cache=null; }
    }

Multi-Threading

It's possible that one thread starts initializing Person while another thread starts initializing Guy. Deadlock is possible, and it does happen in real world. Therefore this design is inherently flawed.

In general, two classes (not necessarily parent-child) having dependency on each other during static initialization is inherently in danger of deadlock.

Application can get away with doing this kind of thing, by carefully triggering initialization in a predictable way at the application start. Still, this is quite dangerous, and it could send someone into days of debugging hell.

like image 36
ZhongYu Avatar answered Sep 20 '22 16:09

ZhongYu