views:

321

answers:

3

I have a situation where a user's code is throwing an IllegalAccessException on a field accessed by reflection. Just before accessing the field, setAccessible(true) is called. So, it would seem to me that this method is silently failing.

Under what situations would this happen? Could this have something to do with a security manager?

Here is the code snippet that is causing the exception:

private static Field levelField;
public int getLevel() {
    try {
        if (levelField == null) {
            levelField = MessageInfo.class.getDeclaredField("level");
            levelField.setAccessible(true);
        }
        return levelField.getInt(this);  // <-- IllegalAccessException thrown here
    } catch (Exception e) {
         handleException(e);
    }
    return ICompilationUnit.NO_AST;
}
A: 

From Java's own documentation for setAccessible():

A SecurityException is raised if flag is true but accessibility of any of the elements of the input array may not be changed (for example, if the element object is a Constructor object for the class Class). In the event of such a SecurityException, the accessibility of objects is set to flag for array elements upto (and excluding) the element for which the exception occurred; the accessibility of elements beyond (and including) the element for which the exception occurred is unchanged.

Unfortunately they don't mention anything about an IllegalAccessException.

Cory Larson
+6  A: 

It shouldn't be a security manager issue - you'd get a SecurityException or subclass.

The code levelField.getInt(*this*) doesn't look right...

You should be passing an instance of MessageInfo as the parameter.

Are you calling this from within the MessageInfo class? (why?!?) or a subclass of MessageInfo? (Trying to make a private field of a superclass act as if it's protected? Does MessageInfo have a getLevel() method? If so, you could call super.getLevel() to get the value instead of attempting it this way.)

If it's not MessageInfo or a subclass, that's your problem - you have the level field of the MessageInfo class and you're attempting to get the value of that field off the current class. Though this should be throwing an IllegalArgumentExeception instead of IllegalAccessException...

If it's really 'IllegalAccessExeception' - try putting some logging inside that if (levelField == null) block - make sure it's really being exececuted. The field is static - there may be some other instance or method setting a value on it.

Nate
Yes, this class is a subclass of the MessageInfo class, which is part of a framework that I am using. So, the level field is a package protected field of MessageInfo with no getter, and so it is otherwise inaccessible to my subclass.This code is working properly for many, many users, but just not this particular one. So, I am wondering if there could be something peculiar about this particular person's setup that is making the call fail here. That is what lead me to think it might possibly be a security manager thing, but I wonder if there could be something else.
Andrew Eisenberg
Perhaps that user has a newer/older version of this framework on the classpath? One where MessageInfo.level doesn't exist or has a different name? P.S. - What framework is it? Might help troubleshoot this...
Nate
Perhaps, but unlikely. The framework I am describing is Eclipse's JDT. And this is code that we have been using since at least 3.3 (before I had been working on the project). And I have tested this up to and including Eclipse 3.5.1. As described by Tom in the accepted answer, I believe it might be a threading issue.
Andrew Eisenberg
+2  A: 

setAccessible is documented to throw a SecurityException. Note that the documentation gives cases where SecurityException will be thrown even if there is no SecurityManager present. Of course it may also fail due to an asynchronous exception: Thread.stop, NIO buffer related exception or a JVM error.

The real problem with this code (other than that it uses reflection) is that there is a field that can be set to be partially initialised. This causes a race condition (you have a mutable static, therefore you need to worry about threads (hint, avoid mutable statics!)). Another thread may call getInt on the same Field before setAccessible is called. And as the original questioner appears to have found out, it isn't exception safe either. It would be much safer and clearer to set up the field in a static initialiser.

Tom Hawtin - tackline
This makes a lot of sense to me. Thanks. I don't know if this is the problem that I am seeing, but it seems possible because I do know that multiple threads have been known to inhabit that part of the code. But even if this is not the real problem, I should be changing my code.
Andrew Eisenberg