On 09/04/2013 06:23 PM, Mandy Chung wrote:
On 9/1/2013 1:16 AM, Nick Williams wrote:
Class<?> getCallerClass(): Retrieves the class of the caller of the
method calling getCallerClass(). This is identical to the new
Reflection#getCallerClass() added in Java 7u25/8.

Class<?> getCallerClass(int): Retrieves the class of the caller n
frames down the stack. This is identical to the old
Reflection#getCallerClass(int) that was deprecated in Java 7u25 and
removed in Java 8.


The other part of this patch is about the ability to obtain the caller
information.  The use case includes Groovy 2.x and GUI app to look up
resource bundles on behalf on the caller and Log4j for logging isolation
depending on its caller (isolation on ClassLoader for the app).

Such methods are caller-sensitive and must obtain the right caller;
otherwise it might get surprises.

The proposed getCallerClass() method, no-arg version, looks reasonable
and it will return the immediate effective caller of the method calling
the getCallerClass method. Use your example (I add the class for the
discussion later).  Let's ignore the @CallerSensitive annotation for now.

     getCallerClass();
     Foo.someMethod1();
     Bar.someMethod2();
     Gee.someMethod3();
     App.actualCallerMethod()

The caller is Bar.someMethod2() and getCallerClass() method will return
Bar.  It will return the same result even if Foo is called via
reflection (the VM already handles this and skips the reflection
machinery).

When security manager is installed, under what condition can Foo class
access Bar class (any permission check)?  What if Bar is in a restricted
package?

The standard way to determine if class A has access to class B is based
on their class loaders and also it is a restricted class (configured in
the package.access property in JRE/lib/security/java.security).  Code
has full access to its own class loader and any class loader that is a
descendent.  There are several methods in java.lang.Class that are
performing this security check:

      * @throw  SecurityException
      *         If a security manager, <i>s</i>, is present and the
caller's
      *         class loader is not the same as or an ancestor of the class
      *         loader for the returning Class object and invocation of
{@link
      *         SecurityManager#checkPackageAccess s.checkPackageAccess()}
      *         denies access to the package of returning Class object

I think the above security check should be applicable to the
getCallerClass method. It means that
Foo can access Bar if Foo's class loader is the same or an ancestor of
Bar's class loader; otherwise, it will perform the package access check.
BTW - including an example in the javadoc would be helpful since the
term "caller" might be confusing.

For the known use cases, they are interested in getting the caller's
class loader.  Class.getClassLoader method has the following security
check:

      * <p> If a security manager is present, and the caller's class
loader is
      * not null and the caller's class loader is not the same as or an
ancestor of
      * the class loader for the class whose class loader is requested,
then
      * this method calls the security manager's {@code checkPermission}
      * method with a {@code RuntimePermission("getClassLoader")}
      * permission to ensure it's ok to access the class loader for the
class.

I wonder if Class instance is really needed while I understand we can't
anticipate all cases.  Most of the caller-sensitive methods in the JDK
only need to get caller's ClassLoader.  There are very few cases that
require the caller class (used in the reflection implementation that I
have yet to understand deeper).   Perhaps all you need is
getCallerClassLoader method?  In that case, you will be able to get the
caller's class loader if you have the access.  This is one thing I'd
like to explore further (either one or both).

Performance - I believe in common cases Foo's class loader should be the
same or ancestor of Bar's clas loader.  Bar calls Foo which should be
visible to Bar's class loader.  It'd be very useful if Groovy and Log4j
can measure the performance overhead with security manager installed
when it's available and see if it is a concern.

About the proposed getCallerClass(int) method to find a frame at a
specific depth is essentially putting back the
sun.reflect.Reflection.getCallerClass(int depth) method.  This method is
very flexible but brittle.  I still think it's more reliable that a
caller-sensitive method should capture the caller and pass it to the
runtime properly. Groovy 3.x doesn't do the stack walk. Groovy 2.x needs
a temporary solution to filter out the intermediate frames of groovy
runtime, would StackTraceFrame.getDeclaringClass be an adequate interim
solution?

More on @CallerSensitive and s.r.Reflection.getCallerClass(int):

We had found severe bugs in the code to call
s.r.Reflection.getCallerClass(int) when prototying the fix for JEP 176
but they were not noticed and no test uncovered them.  e.g. wrong depth
was used (easy to miscount the depth or the depth is modified due to
refactoring but difficult to be caught during review).  As a
caller-sensitive method, it's important to find the right caller;
otherwise unexpected behavior.

In the first prototype, Chris Thalinger did implement JVM_GetCallerClass
to walk past all @CS frames and find the immediate caller.  All methods
in the method chain invoked by the actual caller must be annotated by
@CS which ends up something like the example you used earlier:

@CallerSensitive getCallerClass(int)
@CallerSensitive someMethod1()
@CallerSensitive someMethod2()
@CallerSensitive someMethod3()
@CallerSensitive someMethod4()
actualCallerMethod()

With more analysis, we identified that there was no absolute need (in
the JDK) to walk the stack but instead a more reliable way is to capture
the caller at the entry point of the caller-sensitive methods.  In the
example above, someMethodxx() are internal implementation in the JDK
case and thus we modified them to take the caller class parameter
(instead of looking it up at its execution point).  This greatly
simplifies the VM enforcement to detect if the method calling
JVM_GetCallerClass is a caller-sensitive method.

This seems reasonable on the surface but falls over once you capture the caller for more than one purpose. For example, say a logging framework captures the caller for the purpose of supplementing log information. But you call this logging framework from another framework which uses caller information for another purpose, for example locating resources. The intent here might be to show information from the second framework in the log, however with one universal @CallerSensitive annotation you cannot distinguish which "capture" you want to grab - the second framework, or the caller who called the second framework. However by traversing the stack to a fixed depth, you can do so very definitively (as long as you always know that your internal code does *not* directly call the sensitive method - an easy thing to design for in most frameworks).

In fact you can usually traverse the stack to a fixed depth for this kind of thing, with one key exception that comes up in log frameworks. When you have one log API which forwards to another, you want to capture the "first" caller of any log API. Pursuant to this, most log frameworks have log method variants which accept the fully-qualified class name of that first logger. The moral equivalent to this scenario would likely be to provide an API variant which accepts a Class or ClassLoader (depending on the usage) and a variant which does not and uses a fixed-depth "reach" into the stack instead.

This IMO blows a hole in the whole idea of a single *public* @CS annotation, and in fact in public framework code, a depth indicator seems to be adequate and more or less problem-free for any purpose I've run across. In private JDK code it's a different story because every caller-sensitive usage is in the same code base as the mechanism itself, so there is always a tight control and a guarantee that two different sensitive methods won't interfere at cross-purposes in this way.

My feedback is that the getCallerClass() method seems to be adequate for
all use cases except Groovy 2.x.  I would suggest traversing the stack
trace (with the new getDeclaringClass method) be the alternative to
adding getCallerClass(int) method.  Jochen and others - what do you think?

On the @CS subject, if we keep @CallerSensitive in sun.reflect in JDK8,
the new getCallerClass() method is a caller-sensitive method and if it
is called by the system classes, it will enforce that the caller must be
annotated with @s.r.CS; if it called by non-system classes, the VM will
return the caller and no check on the annotation.  The implication is in
the 292 method handles. The current behavior is that if a MethodHandle
for a caller-sensitive method is requested, it will bind the method
handle with the lookup class (i.e. as if it were called from an
instruction contained in the lookup class).  There is no change to the
current behavior since the current implementation only allows
JVM_GetCallerClass be called from system classes.  If an app (e.g. Foo.m
method) calls the new getCallerClass method, a MethodHandle for Foo.m
will not bind with the lookup class.  This will need to get John Rose
and other 292 members' feedback on it.

This is my thinking so far.  Feedback and comments?

Mandy


--
- DML

Reply via email to