On Mon, 20 Nov 2023 22:08:49 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:

>> Jaroslav Bachorik has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Rewerite the test to use RedefineClassHelper
>
> src/hotspot/share/classfile/classFileParser.cpp line 5579:
> 
>> 5577: 
>> 5578:   if (_methods != nullptr) {
>> 5579:     // Free methods - those methods are not fully wired and miss the 
>> method holder
> 
> How about saying: for methods whose InstanceKlass as method holder is not yet 
> created?

This comment went away with the change @dholmes-ora recommended

> test/hotspot/jtreg/serviceability/jvmti/thread/GetStackTrace/GetStackTraceAndRetransformTest/GetStackTraceAndRetransformTest.java
>  line 53:
> 
>> 51: import java.util.List;
>> 52: import java.util.concurrent.CyclicBarrier;
>> 53: import java.util.concurrent.locks.LockSupport;
> 
> Do you need all these imports?
> 
> There's a simple RedefineClassHelper class that does most of the work, but 
> maybe you need the explicit agent code to reproduce the crash?  See 
> test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/RedefineRunningMethodsWithBacktrace.java
>  as an example.

I did clean up the imports and switched to `RedefineClassHelper`. Thanks for 
pointing that out!

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16662#discussion_r1403064332
PR Review Comment: https://git.openjdk.org/jdk/pull/16662#discussion_r1403054332

Reply via email to