Replacement of sun.reflect.Reflection#getCallerClass

Tue Sep 3 03:04:14 UTC 2013

Hi Nick,

Thanks for the patch.

JEP 176 [1] describes the caller-sensitive method and the need for a 
mechanical checking of caller-sensitive methods.  Also Peter Levart in 
[2] explained the change in MethodHandles.Lookup related to @CS.  I 
assume you understand the rationale behind and the potential security 
issues.  In general defining caller-sensitive API is discouraged.  
Defining a SE supported @CallerSensitive and also getCallerClass API 
poses the risk of "encouraging" developers to implement more @CS methods 
while not fully understand its implication.  It was a non-goal of JEP 
176 to provide @CallerSensitive as a public API and I would suggest not 
to define it as a public API in JDK 8.

While I'll take the time to look at your patch, I would like to restart 
the discussion from the use cases (in which led to what you summarized 
the need of your proposed API [3]):

1. Groovy 1.x and 2.x use the sun.reflect.Reflection.getCallerClass(int 
depth) method to:

  * emulates the way ResourceBundle.getBundle(String, Locale) works.
    Groovy runtime introduces intermediary stack frame between a caller
    and the call to getBundle, these frames needs to be filtered out;
    when looking for the caller classloader.
  * support the annotation @Grab, @Grab allows to specify a dependency
    between a code and a module (using apache ivy under the hood). The
    resolution is done at runtime and require a specific Classloader
    (the GroovyClassLoader), getCallerClass is used to find the class
    loader of the caller, again filtering out the intermediary stack frame.

Groovy 3.x has a different implementation that doesn't need to do stack 
walk to filter its runtime frames and find the caller.

2. Log4j

Log4j offers "extended stack trace" patterns that requires access to a 
Class object when exceptions or stack traces are logged to provide 
additional information for troubleshooting such as the implementation 
version of the Package, code source, etc. It currently uses the 
sun.reflect.Reflection.getCallerClass(int depth) method to find the 
Class object matching the StackTraceElement in a Throwable or stack 
trace. If the sun.reflect.Reflection.getCallerClass(int depth) method 
doesn't exist, it will use SecurityManager.getClassContext().

This approach is not reliable since the HotSpot implementation of 
getCallerClass filters out the frames corresponding to reflection 
machinery (its intent is to find the true caller) whereas a stack trace 
contains all Java frames up to MaxJavaStackTraceDepth (default is 1024). 
When there is no Class object matching a StackTraceElement, it will fall 
back and load the class (does Log4j know which class loader to use?)

Log4j currently works in the absence of the 
sun.reflect.Reflection.getCallerClass(int depth) method but performance 
is a very major issue.

3. APIs on behalf of the caller

For example, locating a resource on behalf of the caller class to avoid 
explicit additional Class and/or ClassLoader parameters.

Please correct/add if I miss anything.

More will be discussed tomorrow.

Mandy
[1] http://openjdk.java.net/jeps/176
[2] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-July/019397.html
[3] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-July/019334.html

On 9/1/2013 1:16 AM, Nick Williams wrote:
> I have completed and am proposing a patch for replacing sun.reflect.Reflection#getCallerClass(...) with a public API in Java 8. I saw no point in duplicating an issue, even though it's over 10 years old, so I'm indicating that this is a patch for 4851444 (http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4851444).
>
> I welcome and solicit support/+1s and a sponsor. Someone about a month ago had mentioned that they would be willing to be a sponsor if the patch looked good, but I can't remember who it was and I can't find the email. I want to say it was someone with RedHat, but my memory could be faulty, so please don't hold it against me if I'm wrong.
>
> *Summary of Changes*
> Added the new class java.lang.StackTraceFrame. It's a virtual mirror of StackTraceElement, except that it contains a Class<?> declaringClass property instead of a String className property. Since the list members expressed reluctance to add methods to Thread and Throwable, StackTraceFrame also contains several static methods for retrieving Classes and StackTraceFrames from the stack. These methods are as follows:
>
> 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.
>
> StackTraceFrame getCallerFrame(): Retrieves the StackTraceFrame of the line of code that called the method calling getCallerClass(). This is similar to the new Reflection#getCallerClass() added in Java 7u25/8, except that it returns a StackTraceFrame.
>
> StackTraceFrame getCallerFrame(int): Retrieves the StackTraceFrame of the caller n frames down the stack. This is similar to the old Reflection#getCallerClass(int), except that it returns a StackTraceFrame.
>
> StackTraceFrame[] getCurrentStackTrace(): Functionally equivalent to Thread#getStackTrace(), except that it returns an array of StackTraceFrames.
>
> StackTraceFrame[] getStackTrace(Throwable throwable): Functionally equivalent to Throwable#getStackTrace(), except that it returns an array of StackTraceFrames. It uses the same save point (backtrace) created when the Throwable is created that Throwable#getStackTrace() uses when it's first called. It caches the array of StackTraceFrames in the Throwable just like the array of StackTraceElements are cached, so that multiple calls for the same Throwable are fast.
>
> As a result of this change, sun.reflect.CallerSensitive has been moved to java.lang.CallerSensitive.
>
> I spent considerable time reviewing, revising, considering, and testing these changes. I included several unit tests that establish the proper behavior. I also spent considerable time benchmarking the changes. While benchmarking, I noticed some things. First, getCallerClass() and getCallerClass(int) are as fast as their counterparts in sun.reflect.Reflection, and they easily inline when appropriate. Second, getCallerFrame() and getCallerFrame(int) are /almost/ as fast as the Class versions, but there is some additional overhead for the construction of the StackTraceFrame. This is minuscule (1,000,000 invocations consume around 500 ms total on my machine). At this point, all of the benchmarking was as expected.
>
> However, I then encountered a surprise. The getCurrentStackTrace() and getStackTrace(Throwable) methods executed (again, 1,000,000 times) in about 70% of the time that Thread#getStackTrace() and Throwable#getStackTrace() did, respectively. Theoretically, they should have executed in the same amount of time, not faster. After extensive analysis, I discovered (what I considered to be) a serious flaw in how the stack trace is filled in within Throwable (which also affects how Thread#getStackTrace() works).
>
> Instead of simply iterating over the entire save point and filling in the Throwable stack trace in native code (which is what I did when I implemented the StackTraceFrame methods), the Java code in Throwable first called a native method to figure out how deep the stack was, then called another native method once for every frame in the stack to retrieve each element individually. This native method that is called repeatedly iterates over the entire backtrace once for each call, stopping only when it finds the matching element (so it's O(1) for the first call, O(2) for the second call, O(3) for the third call, and so on). While my StackTraceFrame methods were iterating over the backtrace exactly 1 time (O(n)), the Throwable methods were iterating over the backtrace 1+(n/2) times (worse than O(nlogn) but not as bad as O(n^2)). This problem would not have been extremely apparent over small stack traces (the 30% improvement was a stack trace of 6 elements), but over a large (200+ elements) stack traces the performance difference would be huge and noticeable. Seeing a room for improvement, I refactored the code that fills in the stack trace for Throwable, improving its performance accordingly to match the performance of the StackTraceFrame methods.
>
> I'm very pleased with how this turned out, and both the unit tests and my extensive functional testing show that the new class and its methods are working great. I just need someone willing to review and sponsor my patch!
>
> *The Code Changes*
> I couldn't get WebRev to run without all kinds of errors. So I ran `hg diff -g` on every repository in the forest with changes. Here are the four patch files for the four repositories that had changes (no other repositories had changes):
>
> https://java.nicholaswilliams.net/Java8GetCallerClass/jdk8.patch
> https://java.nicholaswilliams.net/Java8GetCallerClass/jdk8_jdk.patch
> https://java.nicholaswilliams.net/Java8GetCallerClass/jdk8_hotspot.patch
> https://java.nicholaswilliams.net/Java8GetCallerClass/jdk8_nashorn.patch
>
> I believe I have followed all of the procedures as closely as possible. I await feedback and hope for some support on this, so that we can get a public replacement for this method in Java 8. Let me know if you have any questions.
>
> Thanks!
>
> Nick