Classes on the stack trace (was: getElementClass/StackTraceElement, was: @CallerSensitive public API, was: sun.reflect.Reflection.getCallerClass)

Nick Williams nicholas+openjdk at nicholaswilliams.net
Mon Jul 29 23:21:23 UTC 2013


Okay. Some questions then:

1) Instead of Executable, StackTraceFrame could simply have the method name. Using Executable would be more future-proof (I anticipate someone wanting to know that someday), so if it turns out it's available I'd prefer to use it, but if it's not physically available there we'll have to do without and simply use the method name. Sound reasonable?

2) StackTraceElement is serializable, and Class is serializable, so in theory StackTraceFrame could be serializable and technically Throwable would be serializable. I would have to follow the same serialization rules that Throwable follows for its StackTraceElement[], but it's doable. The question is deserialization, which as you pointed out won't work if a class on the trace isn't available in the ClassLoader that deserialization is occurring in. I would be fine with, and even suggest, making the StackTraceFrame[] transient so that it wasn't serialized with the exception. I think that's better than omitting it from Throwable altogether. (However, I would point out that my original suggestion of adding the Class to StackTraceElement was shot down for serialization reasons, and when I suggested  making it transient that was also shot down.) Being able to obtain the Classes from the stack trace in an exception is one of the key features that Log4j needs, so leaving out any way to obtain this from a Throwable would be disappointing. I think making it transient is OK. I can't imagine still needing the Class from a deserialized Throwable. Thoughts?

3) What about getCallerFrame() and getCallerFrame(int) (as I suggested) AND getCallerClass() and getCallerClass(int) (as you mentioned)? That should create the best of both worlds.

4) What about @CallerSensitive? Part of my proposal was moving that to java.lang. There is a LOT of code that's currently using the private @CallerSensitive. How should I handle that?

5) What about Java 7? As has been widely reported, this change broke a lot of code running on Java 7. We're starting to get near-daily reports of problems now. The proposed idea is to forget about the system property in Java 7 and restore getCallerClass(int) to it's previous operation. How should I proceed on that?

6) Is there, as Paul Benedict suggested, someone willing to sponsor this commit if I create a patch?

7) Any suggestions on what to use and how to get started checking out and creating patches against JDK? I hadn't even heard of Mercurial until I started looking at this issue back in May, and I still don't understand how the project is laid out. Is everything I'm going to need to change within hg.openjdk.java.net/jdk8/jdk8/jdk/? Or are there files in other locations that I'll need to change?

Nick

On Jul 29, 2013, at 5:41 PM, David M. Lloyd wrote:

> I don't see how Throwables could be deserialized with the proposed changes to that class (Class.forName() only works when everything is visible from one class loader - a trivial case which is rarely true in practice, especially in an app server situation).  I think you'd have to leave that part off.
> 
> Other than that, I think StackTraceFrame would definitely be very useful; I think that adding a public getCallerClass(int) method is a lot more realistic though.  Unfortunately I'm not sure the Executable is actually physically available at runtime in hotspot - certainly not as an actual Executable in any case.
> 
> At the very least though, generalizing access to the classes on the call stack would have a lot of uses.  We know that it is possible to access the call stack, given that there are at least two places which do it, so maybe it makes sense to build from there.
> 
> On 07/29/2013 05:16 PM, Nick Williams wrote:
>> I would happily contribute a patch if I were able to. However, I have almost no idea what I'm doing on the native side, and I spent three hours perusing the Mercurial repository and still can't figure out where anything is.
>> 
>> Even so, I would be willing to try. But I can't just blindly make up a patch. I need some kind of guidance as to what you're looking for. Is the API I propose below acceptable? If I propose a patch implementing that API /and it is generally accepted as correct/ is it going to be accepted? Or am I going to spend significant time trying to figure it out with the possibility that it might be rejected merely because of the API.
>> 
>> My goal in this email was to get the community to coalesce around an API. Once that happens, if nobody more qualified than I is willing or has time to implement it I will surely give it a try. But I don't want the start working on something that the community is going to reject, especially having not received any feedback on this API so far.
>> 
>> Nick
>> 
>> On Jul 29, 2013, at 4:59 PM, David M. Lloyd wrote:
>> 
>>> I expect that at this stage, patches speak louder than words :)
>>> 
>>> On 07/29/2013 03:57 PM, Jörn Huxhorn wrote:
>>>> I second all of that.
>>>> 
>>>> It was my understanding that the whole purpose of EA and DP was pinpointing problems like these before they hit consumers. What's the point of them if feedback can't cause the reverting of a breaking change?
>>>> 
>>>> If this is not rectified then the old "Java is slow"-mantra will once again be quite true.
>>>> That, and lots of applications will break since not everyone updating to j7u40 will also update all libraries that currently depend on that method - if a timely fix will exist at all at that point. Throwing UnsupportedOperationException all of a sudden isn't exactly the most subtle way of handling this.
>>>> 
>>>> And please don't argue that everyone should just add that runtime property. This isn't realistic.
>>>> On 29. Juli 2013 at 21:03:10, Nick Williams (nicholas+openjdk at nicholaswilliams.net) wrote:
>>>> 
>>>> Some interesting things to note:
>>>> 
>>>> 1) Someone has been asking for a public API replacement for getCallerClass() since Java 4 (http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4851444). I /would/ have been asking for this for that long, except I haven't needed it until recently and assumed it already existed. .NET has had an API similar to what I proposed below since .NET 1.1.
>>>> 
>>>> 2) http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8014925 discusses the change to getCallerClass(int) and says it will be completely removed (with no system property to re-enable) as early as 7u55. The bug then talks about how feedback needs to be solicited on how the community is using it so that a public API, if needed, can be created. This is that feedback, and the answer appears to be a resounding YES, a public API needs to replace it because this change is disastrous for many projects. As Paul says, at the very least getCallerClass(int) should keep working like it does in <7u25 (with or without a system property) through the rest of Java 7 and Java 8 if a replacement public API is not adopted.
>>>> 
>>>> 3) The performance test numbers below don't reflect the fact that some use cases (like Log4j) have to use Thread/Throwable#getStackTrace() AND Reflection/SecurityManager. These use cases need the method name/line number AND the Class instance. If Reflection#getCallerClass() is taken away from us and SecurityManager#getClassContext() is unavailable (possible if an existing SecurityManager prohibits instantiating the SecurityManager class), these use cases have to call getStackTrace() and THEN Class#forName() for each StackTraceElement. I'm sure you can see where performance becomes a serious issue at that point. The proposed API below was designed to solve all of the uses cases that have been discussed in a well-performing way.
>>>> 
>>>> Nick
>>>> 
>>>> On Jul 29, 2013, at 10:47 AM, Jörn Huxhorn wrote:
>>>> 
>>>> The numbers are from this link: http://stackoverflow.com/questions/421280/in-java-how-do-i-find-the-caller-of-a-method-using-stacktrace-or-reflection
>>>> 
>>>> Even if this benchmark suffers from micro-benchmark issues:
>>>> a slow-down of 10x would be bad, a slow-down of 100x would be a catastrophe.
>>>> 
>>>> I'd suggest to at least postpone the UnsupportedOperationException change until we find a suitable replacement. This change will also break existing Groovy scripts. See http://jira.codehaus.org/browse/GROOVY-6279 - but there are other issues as well.
>>>> 
>>>> Cheers,
>>>> Jörn.
>>>> 
>>>> On 29. Juli 2013 at 16:49:02, Nicholas Williams (nicholas+openjdk at nicholaswilliams.net) wrote:
>>>> 
>>>> I wasn't the one who ran the test, so I don't know for sure. My theory
>>>> was that getCallerClass() returns a single frame, but the
>>>> SecurityManager must allocate an array of appropriate size (which
>>>> involves some overhead) and then return all of the frames. I chalked
>>>> the difference up to that. My conclusion from the data was: If you
>>>> need a whole stack, SecurityManager is clearly the best option. If you
>>>> need a single frame, getCallerClass() is the only option that makes
>>>> any sense.
>>>> 
>>>> On Mon, Jul 29, 2013 at 8:21 AM, David M. Lloyd <david.lloyd at redhat.com> wrote:
>>>> I find it very interesting that reflection is no less than two orders of
>>>> magnitude faster than the security manager solution. How big was the stack
>>>> in these tests? It makes me wonder if maybe the implementation of the
>>>> security manager's getContext() method should be reevaluated a bit.
>>>> 
>>>> 
>>>> On 07/29/2013 07:53 AM, Nick Williams wrote:
>>>> 
>>>> Just so that everyone understands how important this subject is, this
>>>> change to getCallerClass(...) is being labeled a "disaster" for logging
>>>> frameworks everywhere. Here's a benchmark for getting Classes from the
>>>> following methods:
>>>> 
>>>> 1,000,000 calls of all alternatives were measured as follows :
>>>> Reflection: 10.195 ms.
>>>> Current Thread StackTrace: 5886.964 ms.
>>>> Throwable StackTrace: 4700.073 ms.
>>>> SecurityManager: 1046.804 ms.
>>>> 
>>>> 
>>>> My goal here is to get the entire list engaged in coming up with the right
>>>> solution. We (the community) can't afford for Java 8 not to have an
>>>> equivalent replacement for getCallerClass().
>>>> 
>>>> Nick
>>>> 
>>>> On Jul 27, 2013, at 2:01 PM, Nick Williams wrote:
>>>> 
>>>> All,
>>>> 
>>>> In the last two months, there have been a number of discussions
>>>> surrounding stack traces, Classes on the stack trace, and caller classes
>>>> [1], [2], [3]. These are all related discussions and the solution to them is
>>>> equally related, so I wanted to consolidate it all into this one discussion
>>>> where I hope we can finalize on a solution and get it implemented for Java
>>>> 8.
>>>> 
>>>> In a nut shell, here are the underlying needs that I have seen expressed
>>>> through many, many messages:
>>>> 
>>>> - Some code needs to get the Class of the caller of the current method,
>>>> skipping any reflection methods.
>>>> - Some code needs to get the Class of the caller /n/ stack frames before
>>>> the current method, skipping any reflection methods.
>>>> - Some code needs to get the current stack trace, populated with Classes,
>>>> Executables, file names, line numbers, and native flags instead of the
>>>> String class names and String method names in StackTraceElement. This
>>>> /should/ include any reflection methods, just like StackTraceElement[]s.
>>>> - Some code needs to get the stack trace from when a Throwable was
>>>> created, populated with Classes, Executables, file names, line numbers, and
>>>> native flags instead of the String class names and String method names in
>>>> StackTraceElement. This /should/ include any reflection methods, just like
>>>> StackTraceElement[]s.
>>>> - There needs to be a reflection way to achieve all of this since some
>>>> libraries (e.g., Log4j) need to be compiled against Java 6 but run on 7 and
>>>> 8 (and thus can't use @CallerSensitive).
>>>> 
>>>> I believe the solutions to these needs are all related. Importantly, I
>>>> think it is very important that action be taken in Java 8 due to the changes
>>>> made to sun.reflect.Reflection#getCallerClass(...). While we all understand
>>>> that relying on private sun.* APIs is not safe, the fact is that many people
>>>> have relied on sun.reflect.Reflection#getCallerClass(...) due to the fact
>>>> that there is simply no other way to do this in the standard API. This
>>>> includes Log4j 2, Logback, SLF4j, and Groovy, some features of which will
>>>> stop working correctly in Java 7 >= u25.
>>>> 
>>>> I would point out that this could all easily be solved simply by adding a
>>>> getElementClass() method to StackTraceElement, but there was strong
>>>> opposition to this, largely due to serialization issues. Since that is
>>>> apparently not an option, I propose the following API, based on the various
>>>> discussions in the last two months, StackTraceElement, and the API that .NET
>>>> provides to achieve the same needs as listed above:
>>>> 
>>>> CallerSensitive.java:
>>>> package java.lang;
>>>> 
>>>> /** Previously private API, now public */
>>>> public @interface CallerSensitive {
>>>> ...
>>>> }
>>>> 
>>>> StackTraceFrame.java:
>>>> package java.lang;
>>>> 
>>>> import java.util.Objects.
>>>> 
>>>> public final class StackTraceFrame {
>>>> private final Class<?> declaringClass;
>>>> private final Executable executable;
>>>> private final String fileName;
>>>> private final int lineNumber;
>>>> 
>>>> public StackTraceFrame(Class<?> declaringClass, Executable
>>>> executable, String fileName, int lineNumber) {
>>>> this.declaringClass = Objects.requireNonNull(declaringClass,
>>>> "Declaring class is null");
>>>> this.executable = Objects.requireNonNull(executable, "Executable
>>>> is null");
>>>> this.fileName = fileName;
>>>> this.lineNumber = lineNumber;
>>>> }
>>>> 
>>>> public Class<?> getDeclaringClass() {
>>>> return this.declaringClass;
>>>> }
>>>> 
>>>> public Executable getExecutable() {
>>>> return this.executable;
>>>> }
>>>> 
>>>> public String getFileName() {
>>>> return this.fileName;
>>>> }
>>>> 
>>>> public int getLineNumber() {
>>>> return this.lineNumber;
>>>> }
>>>> 
>>>> public boolean isNative() {
>>>> return this.lineNumber == -2;
>>>> }
>>>> 
>>>> public String toString() { /* Same as StackTraceElement */ }
>>>> public boolean equals() { /* Ditto */ }
>>>> public int hashCode() { /* Ditto */ }
>>>> 
>>>> /** Uses @CallerSensitive */
>>>> public static native StackTraceFrame getCallerFrame();
>>>> 
>>>> /** Works like Java < 7u25 sun.reflect.Reflection#getCallerClass()
>>>> */
>>>> public static native StackTraceFrame getCallerFrame(int skipFrames);
>>>> 
>>>> public static native StackTraceFrame[] getCurrentStackTrace();
>>>> }
>>>> 
>>>> Throwable.java:
>>>> package java.lang;
>>>> 
>>>> ...
>>>> 
>>>> public class Throwable {
>>>> ...
>>>> public synchronized Throwable fillInStackTraceFrames() { ... }
>>>> 
>>>> private native Throwable fillInStackTraceFrames(int dummy);
>>>> 
>>>> public StackTraceFrame[] getStackTraceFrames() {
>>>> return this.getOurStackTraceFrames().clone();
>>>> }
>>>> 
>>>> private synchronized StackTraceFrame[] getOurStackTraceFrames() {
>>>> ... }
>>>> ...
>>>> }
>>>> 
>>>> Furthermore, I propose that we restore the behavior of
>>>> sun.reflect.Reflection#getCallerClass(int) /just for Java 7/ since the
>>>> proposed above solution cannot be added to Java 7.
>>>> 
>>>> I would love if we could quickly coalesce around this solution or a
>>>> derivative thereof so that it can be implemented before Feature Complete.
>>>> The absence of any replacement or alternative for
>>>> sun.reflect.Reflection#getCallerClass(int) will be a serious issue in Java 8
>>>> that will cause hardships for many projects.
>>>> 
>>>> [1]
>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/018049.html
>>>> [2]
>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-June/018349.html,
>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-July/019098.html
>>>> [3]
>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2013-July/018855.html
>>>> 
>>>> 
>>>> 
>>>> 
>>>> --
>>>> - DML
>>>> 
>>> 
>>> 
>>> --
>>> - DML
>> 
> 
> 
> -- 
> - DML




More information about the core-libs-dev mailing list