ansaurus

Question

Java: how to avoid circual references when dumping object information with reflection?

Answer 1

+3 A:

+1 for using IdentityHashMap to fix the problem.

The reason for this is that your method is currently dependent upon how each visited object's class implements equals, since List.contains(Object) uses equals as the basis for comparison. If a class's equals() method is broken, and incorrectly returns false even when passed itself as the comparison object, then you will get an infinite loop because the call to List.contains always returns false and that object subgraph is always traversed for that type of object.

Additionally, if you have two or more objects that are distinct instances, but are considered equal by value (i.e. equals returns true), only one of these will be written out. Whether this is desirable or a problem depends upon your use case.

Using an IdentityHashMap will avoid both of these problems.

~~An aside - if you want to indent according to the call depth, don't forget to increment callCount on recursive calls to dumpFields.~~

EDIT: I think the code is working correctly. The problem is that you really are getting a stack overflow. This will happen if you have a large object graph. For example, imagine a linked list of 3000 elements. That will involve 3000 recursive calls, which I'm pretty sure will blow the stack with the default stack size.

To fix this, you either increase the size of the stack (vmarg -Xss) to be large enough to handle your anticipated object graph size (not a robust solution!) or, replace use of the stack with an explicit data structure.

Instead of the stack, create a work list. This list holds objects that you've seen but not yet processed. Rather than recursively call dumpFields, you simply add the object to your work list. The main body of the method is a while loop that iterates as long as there are items in the list.

E.g.

class CallLevel
{
    CallLevel(Object target, int level) {
        this.target = target; this.level = level;
    }
    Object target;
    int level;
}
public static String dumpFields(Object start)
{
    List<CallLevel> workList = new ArrayList<CallLevel>();
    workList.add(new Calllevel(start,0));
    Map idHashMap = new IdentityHashMap();

    while (!workList.isEmpty()) {
        CallLevel level = workList.removeAt(workList.size()-1);
        Object o = level.object;
    //add this object to the exclude list to avoid circual references in the future
    idHashMap.put(, o);

    //setup string buffer and add fields
    StringBuffer tabs = new StringBuffer();
    int callCount = level.level;
    for (int k = 0; k < callCount; k++)
    {
        tabs.append("\t");
    }
    callCount++;
    StringBuffer buffer = new StringBuffer();
    Class oClass = o.getClass();
    if (oClass.isArray()) {         
        buffer.append("\n");
        buffer.append(tabs.toString());
        buffer.append("[");
        for (int i = 0; i < Array.getLength(o); i++)
        {
            if (i < 0) buffer.append(",");
            Object value = Array.get(o, i);

            if (value != null)
            {
                if (idHashMap.containsKey(value))
                {
                    buffer.append("circular reference");
                }
                else if (value.getClass().isPrimitive() || value.getClass() == java.lang.Long.class || value.getClass() == java.lang.String.class || value.getClass() == java.lang.Integer.class || value.getClass() == java.lang.Boolean.class)
                {
                    buffer.append(value);
                }
                else
                {
                    workList.add(new Calllevel(value, callCount));
                }
            }
        }
        buffer.append(tabs.toString());
        buffer.append("]\n");
    }
    else
    {           
        buffer.append("\n");
        buffer.append(tabs.toString());
        buffer.append("{\n");
        while (oClass != null)
        {               
            Field[] fields = oClass.getDeclaredFields();
            for (int i = 0; i < fields.length; i++)
            {
                if (fields[i] == null) continue;

                buffer.append(tabs.toString());
                fields[i].setAccessible(true);
                buffer.append(fields[i].getName());
                buffer.append("=");
                try
                {
                    Object value = fields[i].get(o);
                    if (value != null)
                    {
                        if (idHashMap.containsKey(value))
                        {
                            buffer.append("circular reference");
                        }
                        else if ((value.getClass().isPrimitive()) || (value.getClass() == java.lang.Long.class) || (value.getClass() == java.lang.String.class) || (value.getClass() == java.lang.Integer.class) || (value.getClass() == java.lang.Boolean.class))
                        {
                            buffer.append(value);
                        }
                        else
                        {
                            workList.add(new CallLevel(value, callCount));
                        }
                    }
                }
                catch (IllegalAccessException e)
                {
                    System.out.println("IllegalAccessException: " + e.getMessage());
                }
                buffer.append("\n");
            }
            oClass = oClass.getSuperclass();
        }
        buffer.append(tabs.toString());
        buffer.append("}\n");
    }
    return buffer.toString();

EDIT2: I've just ran the code to see what happens. There are 3 main changes needed to make this work:

The test for primitive types should be the first test (the first of the 3 if statements.) the second else if is then the test against the exclude map.
The test for primitive types needs to include checks for ALL primitive classes. You have currently a few tests, but float, double, byte, short and long are missing.
Skip over static fields, check Modifier.isStatic(field.getModifiers()).

The reason primitive tests should happen first is that with reflection, the primitive type is boxed using a new instance of the corresponding class (e.g. for a double field a new Double is created - this is a simpliciation - the JDK will actually reuse some objects, see the sources for Integer.valueOf() but in general a new object is created when a primitive is boxed.) As these primities generate unique new objects, there is little point checking these against the exclude map. Hence, put the primite test first. Incidentally, the check value.getClass().isPrimitive() will always return false - the boxed type is never a primitive type. You can instead use the declared type of the field, e.g. field.getType().isPrimitive().

The test against classes of boxed primitives, must include tests for all boxed primive classes. If it doesn't, then these new boxed objects will continue to be created, found not to be already excluded (since they are new instances) and added to the work list. This becomes a runaway problem - the static public final constants like MAX_VALUE cause generation of more instances, which are added to the list, and reflection of the fields of those objects cause more values etc... The fix is to ensure all primitive types are tested for (or use the isPrimitive on the field type, not the returned object type.)

Not outputting static fields will serve as an ancilliary fix to the problem above, but more importantly it will save your output from being cluttered with unnecessary details.

mdma 2010-04-30 19:23:50

callCount does increment, it's the third line in the method. I'll try using the IdentityHashMap (I don't know how, so I will have to find out) and post back results.

Tom 2010-04-30 20:06:14

It's fairly straightforward - replace ArrayList, with Map, and initialize when null with a new IdentityHashMap. Rather than calling List.add(o) to add excluded object, use Map.put(o,o) (the key and the value are the same.)Ah..I missed the callCount increment.

mdma 2010-04-30 20:09:32

Alright, updated that. Unfortunately I still get a StackOverflow error. Please see my updated original post. Thanks.

Tom 2010-04-30 20:21:10

Thanks again for the update. Unfortunately I am not quite there yet, could you maybe check my updated original post? Thanks a lot.

Tom 2010-04-30 23:52:50

Thanks for the second edit. I was trying to rewrite the method, however I get stuck at the part where you want to check for primitive types before checking if it is an array or not. Because, the primitive type check is done on the fields, not this object. The method of getting the fields is dependent on the object being an array or not. How would you do this?

Tom 2010-05-01 12:44:51

Nevermind, I misread your edit. I'm rewriting the function now, will report back when done.

Tom 2010-05-01 12:52:53

I updated my original post, if you could have a look it would be appreciated. Thanks.

Tom 2010-05-01 13:05:46

The call to isPrimitive on the array index value is not correct, for the reasons I give in the answer (it will be a boxed type, not a primitive.) You can either explicitly check against all classes (Integer.class, Double.class etc...) or call o.getClass().getComponentType().isPrimitive().

mdma 2010-05-01 16:15:02

I can't call getComponentType on an array element, so I used the manual method. Unfortunately I still get the OutOfMemory error. Did you try to run the code?

Tom 2010-05-01 22:59:26

I did run the code. I serialized a HashMap with a few integers in it. I hit the OutOfMemory problem and then fixed it. getComponentType() is called on the array - I said "o.getClass().getComponentType()". In the code "o" is the array, "value" is the array element. You can't use isPrimitive on the array element - the value will be a boxed type - Integer, Double etc. not int, double, which are the corresponding primitives. isPrimitive() will always return false for the value, but not for the array component type. I've said all this before, did you read my answer? :-)

mdma 2010-05-02 00:13:08

Of course I read your answer, you're the only one here who could help me out here and I'm really grateful. I figured the getComponentType would resolve the boxed type issue (which I don't really understand). But now it seems like you want to know if the array is a primitive type? Why do you want to do this when you want to store the value of the array, not the array itself? I'm using my method on a class with a few GUI elements, arrays and integers and then storing the output in a TextArea. When I return the output in the console, I don't get the OutOfMemory error. However...

Tom 2010-05-02 10:44:25

However... the output does not seem to be correct. I'm getting a whole lot of whitespace and an insanely long return string.

Tom 2010-05-02 10:45:00

Ok, good news that you have now something working. That the formatting is not as expected is really a different problem. I've not focused on the structure or output issues since your primary focus was on fixing the Stack overflow/OutOfMmeoryError (as you mention in your first comment.) I suggest you now refactor the code to get the basic strucutre cleaner and then focus on the formatting.

mdma 2010-05-02 12:09:05

Alright, thanks for the help (PS. did you just double your reputation in 2 days? :)

Tom 2010-05-03 09:57:08

Yes, I guess I did :D Reflection does a lot behind the scenes, and boxing primitives is particularly nasty wrt creating surprises. But now we've sorted that out, the code should appear more transparent to you, and tweaking the formatting to get what you want should not pose any difficult challenges.

mdma 2010-05-03 11:20:38

ansaurus

tags:

views:

answers:

Java: how to avoid circual references when dumping object information with reflection?

related questions