Hi Stefan,

This part of the test:

  86     private static void doClassUnloading() {
  87         // This will clean out old, unused redefined methods.
  88         System.gc();
  89     }

seems to make assumptions about System.gc() and class unloading. Are we relying on knowledge of hotspot internals here?

I don't have a good suggestion to avoid this. This topic came up on another thread today regarding weak references. For that case we can typically add a loop and sleep until we see the reference clear, but here ... how can you check that class unloading occurred? Actually is that really what is happening - I don't see how any class unloading will occur here. I can imagine that GC might cleanup unreferenced methods.

David

On 1/02/2013 9:27 PM, Stefan Karlsson wrote:
On 2013-02-01 12:17, Stefan Karlsson wrote:
On 2013-02-01 12:11, serguei.spit...@oracle.com wrote:
On 2/1/13 1:57 AM, Stefan Karlsson wrote:
On 2013-02-01 10:22, serguei.spit...@oracle.com wrote:
Nice test!
It looks good.
Thanks for reviewing!

As the original bug and the test are non-trivial, it'd make sense
to add a comment to
the class RedefineMethodInBacktraceApp and explain a little bit
what the test is doing,
and what behavior is expected.
http://cr.openjdk.java.net/~stefank/8006506/webrev.04/

Tell me if you think this is good enough.

It is good.

Nit: it'd be enough if it is more specific. :)
This method is a key point:
   90     private static void touchRedefinedMethodInBacktrace(Throwable 
throwable) {
   91         throwable.getStackTrace();
   92     }
Is it true that the test expects the getStackTrace() does not crash nor
throw an exception which would happen if the old/obsolete method is
gc'ed?

I see. I'll add a comment that we shouldn't crash.

http://cr.openjdk.java.net/~stefank/8006506/webrev.05/

StefanK


Thanks,
StefanK


Thanks,
Serguei



thanks,
StefanK


Thanks,
Sergueri


On 2/1/13 12:13 AM, Stefan Karlsson wrote:
http://cr.openjdk.java.net/~stefank/8006506/webrev.03/

1) Reverted the ProblemList change, since the fix has already
propagaged to jdk8/tl
2) Renamed do_redefine -> doRedefine
3) Updated the .sh file with the bug number of the original CR
instead of the test CR.

thanks,
StefanK

On 2013-01-22 14:11, Stefan Karlsson wrote:
http://cr.openjdk.java.net/~stefank/8006506/webrev.00/

This test provokes the JVM crash described in bug: JDK-7174978.

I intend to push this to:
http://hg.openjdk.java.net/jdk8/tl/jdk

thanks,
StefanK








Reply via email to