Use of setAccessible causes problems with JPMS

[richard-melvin]

Updating my client code to JDK13, a template like:

$if (arguments.empty)$, where arguments is Collections.unmodifiableList(arguments);

gets an exception

internal error: java.lang.reflect.InaccessibleObjectException: Unable to make public boolean java.util.Collections$UnmodifiableCollection.isEmpty() accessible: module java.base does not "opens java.util" to module ST4
    at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:344)
    at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:284)
    at java.base/java.lang.reflect.Method.checkCanSetAccessible(Method.java:198)
    at java.base/java.lang.reflect.Method.setAccessible(Method.java:192)
    at ST4@4.3/org.stringtemplate.v4.misc.ObjectModelAdaptor.tryGetMethod(ObjectModelAdaptor.java:138)
    at ST4@4.3/org.stringtemplate.v4.misc.ObjectModelAdaptor.findMember(ObjectModelAdaptor.java:118)
    at ST4@4.3/org.stringtemplate.v4.misc.ObjectModelAdaptor.getProperty(ObjectModelAdaptor.java:72)

Since JDK9, Method.setAccessible can throw InaccessibleObjectException, which is not caught.

This fires in this case because UnmodifiableCollection is not a public class, even though the method isEmpty is public and there would be no problem in calling it.

I think the code either needs to catch InaccessibleObjectException, or maybe just RuntimeException, or work out how to avoid calling that function when it is not required.

[Clashsoft]

InaccessibleObjectException was added in JDK 9, which is problematic to use since the codebase is stuck on Java 6.

[richard-melvin]

Hence catching RuntimeException.

[Clashsoft]

In https://github.com/antlr/stringtemplate4/blob/e38a1c81422317d04908c33dfaa5df6c42753653/src/org/stringtemplate/v4/misc/ObjectModelAdaptor.java#L82 it already treats "any Exception when calling" as "no such property". I think treating "any Exception on method lookup" the same wouldn't be far fetched.

[richard-melvin]

Note that the difference in this case is that just calling the method, without first calling setAccessible, would work.

e,g.

  protected static Field tryGetField(Class<?> clazz, String fieldName)
  {
    try
    {
      Field field = clazz.getField(fieldName);
      if (field != null)
      {

        try
        {
          field.setAccessible(true);
        }
        catch (RuntimeException e)
        {
          // ignore as it may already be accessible
        }
      }

      return field;
    }
    catch (NoSuchFieldException | SecurityException ex)
    {
      // return null to try other options

      return null;
    }

  }

[richard-melvin]

Turns out that is not enough, you actually need something like:

  protected static Method tryGetMethod(Class<?> clazz, String methodName)
  {
    try
    {
      Method method = clazz.getMethod(methodName);
      if (method != null)
      {
        if (isAccessible(method))
        {
          return method;
        }
        for (Class<?> baseInterface : clazz.getInterfaces())
        {
          Method baseMethod = baseInterface.getMethod(methodName);
          if (baseMethod != null && isAccessible(baseMethod))
          {
            return baseMethod;
          }
        }
        if (clazz.getSuperclass() != null)
        {
          Method baseMethod = tryGetMethod(clazz.getSuperclass(), methodName);
          if (baseMethod != null)
          {
            return baseMethod;
          }
        }

      }
      return null;
    }
    catch (NoSuchMethodException ex)
    {
      return null;
    }

  }

  /**
   * Access check for method, in a way that is portable to JDK 8 and 9+
   * @param method
   * @return true if accessible
   */
  private static boolean isAccessible(Method method)
  {
    try
    {
      method.setAccessible(true);
    }
    catch (RuntimeException e)
    {
      return false;
    }
    return true;
  }