RFR: 8173898: StackWalker.walk throws InternalError if called from a constructor invoked through reflection.

Mandy Chung mandy.chung at oracle.com
Tue Feb 7 20:03:47 UTC 2017


> On Feb 7, 2017, at 5:10 AM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> 
> Hi Mandy,
> 
> On 06/02/17 00:16, Mandy Chung wrote:
>> Hi Daniel,
>> 
>> Thanks for the patch and uncover that Constructor::newInstance is not covered by SHOW_REFLECT_FRAMES.
>> 
>> The change looks okay.  The javadoc of SHOW_REFLECT_FRAMES can be clarified to include Constructor::newInstance.  As for the test, can you separate this in a new test to test showing and hiding reflecton frames and also getCallerClass?
> 
> Please find below a new webrev that incorporates your feedback.
> I have added a new test, and also tweaked the API documentation
> in StackWalker.java as you suggested:
> http://cr.openjdk.java.net/~dfuchs/webrev_8173898/webrev.03/
> 

I think this sentence is not needed.
 216          * <p>A {@code StackWalker} with this {@code SHOW_REFLECT_FRAMES} option
 217          * will show all reflection frames.

Basic.java test - the @bug already includes the bug number.  line 67-69 can be removed.  

Minor comments: StackFrame.getClassName() can replace StackFrame.getDeclaringClass().getName().  Or if you want to compare package name, StackFrame.getDeclaringClass().getPackageName is an alternative.

Nit: what about renaming SimpleObj to ConstructorNewInstance? 
SimpleObj::found should probably name as `framesInUnnamedOrReflectionPackage`.

Nit: ReflectionFrames.java test - the test is a bit hard maybe SimpleObj can be renamed to TestHelper and `found` be `result`.   Just trying to make the test easier to read and understand.  You may have better idea.

Otherwise looks good.

Mandy



More information about the core-libs-dev mailing list