CODEWITHSUNDEEP
javastaticclassloader
codebreaker
codebreaker

Reputation: 813

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?

Upvotes: 8

Views: 516

Answers (4)

diufanman
diufanman

Reputation: 201

change 

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

to 

public static final List<Guy> GUYS = Collections.unmodifiableList(Arrays.asList(new Guy[] {Guy.TOM, Guy.DICK, Guy.HARRY}));

should work (I tested using JDK 8), but is it still meet your requirement?

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 = Collections.unmodifiableList(Arrays.asList(new Guy[] {Guy.TOM, Guy.DICK, Guy.HARRY}));
}

Upvotes: 1

scottb
scottb

Reputation: 10084

Is the requirement to use the Person abstract class absolute (i.e. is it already a part of legacy code)?

If not then as a recommendation, I suggest that you could refactor the design of your data model into a Guy enum class that implements a Person interface. There are many advantages to this design over the class-based one.

public enum Guy implements Person {

    TOM {
        @Override
        public void personMethod1(...) {...} // constant-specific
            :
            :
    },
    DICK {
        @Override
        public void personMethod1(...) {...} // constant-specific
            :
            :
    };

    @Override
    public int personMethod2(...) {...} // general to the enum class
        :
        :
}

Note that in this case there is no reason to define a List instance with the constant references as the enum API provides the GUY.values() method.

It would solve all your initialization woes while still enabling multiple implementations of Person. The downside is in code duplication. The methods in Person will need to be implemented in every enum class (but if there are many methods you can always create a private static helper class and forward method calls to it.)

Upvotes: 1

ZhongYu
ZhongYu

Reputation: 19682

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.

Upvotes: 2

Dmitry Grigoryev
Dmitry Grigoryev

Reputation: 3203

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();
    }
}

Upvotes: 3

Related Questions