On 4 okt 2013, at 00:49, Coleen Phillmore <coleen.phillim...@oracle.com> wrote:
> > Thanks Dan - > > On 10/3/2013 4:28 PM, Daniel D. Daugherty wrote: >> > open webrev at http://cr.openjdk.java.net/~coleenp/8025238_jdk >> >> test/java/lang/instrument/RedefineMethodInBacktrace.sh >> No comments. >> >> test/java/lang/instrument/RedefineMethodInBacktraceApp.java >> line 78: t.setDaemon(true); >> Why make the target thread a daemon? Wouldn't it be a more >> complete test to do Thread.join() of that thread just to >> be sure that the target thread has finished (and is not >> stuck)? > > Staffan, can you answer this? You could do it either way. I don't have a strong opinion. It's possible that the deamonization is a leftover from one of my iterations of the test. /Staffan > > Coleen > >> >> test/java/lang/instrument/RedefineMethodInBacktraceTargetB.java >> test/java/lang/instrument/RedefineMethodInBacktraceTargetB_2.java >> Nice sync logic with the test driver. >> >> Thumbs up. >> >> Dan >> >> >> On 10/3/13 12:02 PM, Coleen Phillimore wrote: >>> Summary: Redefined class in stack trace may not be found by method_idnum so >>> handle null. >>> >>> This is a simple change. I had another change to save the method name (as >>> u2) in the backtrace, but it's not worth the extra footprint in backtraces >>> for this rare case. >>> >>> The root problem was that we save method_idnum in the backtrace (u2) >>> instead of Method* to avoid Method* from being redefined and deallocated. >>> I made a change to InstanceKlass::method_from_idnum() to return null rather >>> than the last method in the list, which causes this crash. Dan and I went >>> down the long rabbit-hole of why method_idnum is changed for obsolete >>> methods and we think there's some cleanup and potential bugs in this area. >>> But this is not that change. I'll file another bug to continue this >>> investigation for jdk9 (or 8uN). >>> >>> Staffan created a test - am including core-libs for the review request. >>> Also tested with all of the vm testbase tests, mlvm tests, and >>> java/lang/instrument tests. >>> >>> open webrev at http://cr.openjdk.java.net/~coleenp/8025238/ >>> bug link https://bugs.openjdk.java.net/browse/JDK-8025238 >>> >>> test case for jdk8 repository: >>> >>> open webrev at http://cr.openjdk.java.net/~coleenp/8025238_jdk >>> >>> Thanks, >>> Coleen >>> >>> >> >
