Thoughts on adding getElementClass() method to StackTraceElement?

[Nick Williams]

On Jun 16, 2013, at 1:25 AM, Jeroen Frijters wrote:

> Nick Williams wrote:
>> I'm going to stick by my original assessment that I'm not convinced
>> there's a security issue. It's possible that getClassContext() filters
>> out classes the caller can't access, but nothing in the documentation
>> indicates that's the case, so I'm operating under the assumption that it
>> doesn't.
> 
> SecurityManager.getClassContext() is not available to unpriviliged callers, so I don't think this a valid argument.

Can you define what "is not available to unprivileged callers" means a little better? We use it in Log4j 2 when getCallerClass() isn't available, and it works just fine. I took a look at the Java code, and it's native, so there's no check there. (You can't instantiate a SecurityManager unless there's not currently a SecurityManager installed _OR_ the installed SecurityManager allows the creation (installation privilege isn't checked) of another SecurityManager. Is that what you're talking about?) I took a look at the native code, and I don't see anything there that would represent a privilege check that I recognized, but if I understood what you meant a little better I might see where I'm wrong.

> Given the security implications, the serialization issue and the need for weak references, it seems to me that adding this to Throwable would be way too expensive. 

The only expense I'm seeing here is the security implications, and I'm still not convinced that they exist. We have all agreed now that making this transient is okay, and adding the transient keyword to a field only takes 5 seconds. And I certainly don't believe that declaring the field as "WeakReference<Class<?>> elementClass" instead of "Class<?> elementClass" takes more than 30 seconds longer, but maybe I'm missing something, so that's not expensive either.

Also, to be clear, I'm not suggesting adding it to Throwable specifically, I'm suggesting adding it to StackTraceElement (which would apply anywhere that a stack trace is filled in, including Throwable). A stack trace is filled in with one native code method, so updating that method will make this work in Throwable, Thread, and anywhere else that you can get a stack trace. I know this is a nitpicky distinction, but it could be important.

> 
> Adding a new API to collect a stack trace seems like a far better approach.
> 
> Here's my off the cuff proposal:
> 
> public final class StackFrame {
>  public Executable method();
>  public String getFileName();
>  public int getLineNumber();
> 
>  public static StackFrame[] capture(int skipFrames, int maxLength, boolean includeSourceInfo);
> }
> 

I'm not opposed to the StackFrame class as suggested, it just doesn't meet my needs. At all. I can already successfully get this information using the stack trace and getClassContext(), so I wouldn't be compelled to switch. I need to efficiently get the Class from a stack trace generated by a Throwable, which this does not solve. Right now I'm having to resort to getting that Class using inefficient methods.

Nick