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